Address review comments about tests and names

CarolineConcatto · CarolineConcatto · commit 13c7470bde9d · 2025-07-15T12:50:07.000Z
This patch is removing the keyworkd and token inaccessibleWrite and
inaccessibleRead. It is using Read and Write to set the Target Memory
Location.

Adding "aarch64" in front of the target specific memory locations
diff --git a/llvm/include/llvm/AsmParser/LLToken.h b/llvm/include/llvm/AsmParser/LLToken.h
@@ -202,15 +202,13 @@ enum Kind {
   kw_readwrite,
   kw_argmem,
   kw_inaccessiblemem,
-  kw_fpmr,
-  kw_za,
+  kw_aarch64_fpmr,
+  kw_aarch64_za,
   kw_errnomem,
 
   // Legacy attributes:
   kw_argmemonly,
   kw_inaccessiblememonly,
-  kw_inaccessiblereadmemonly,
-  kw_inaccessiblewritememonly,
   kw_inaccessiblemem_or_argmemonly,
   kw_nocapture,
 
diff --git a/llvm/include/llvm/IR/IntrinsicsAArch64.td b/llvm/include/llvm/IR/IntrinsicsAArch64.td
@@ -9,6 +9,7 @@
 // This file defines all of the AARCH64-specific intrinsics.
 //
 //===----------------------------------------------------------------------===//
+
 let TargetPrefix = "aarch64" in {
 
 def int_aarch64_ldxr : Intrinsic<[llvm_i64_ty], [llvm_anyptr_ty],
diff --git a/llvm/include/llvm/Support/ModRef.h b/llvm/include/llvm/Support/ModRef.h
@@ -147,18 +147,6 @@ template <typename LocationEnum> class MemoryEffectsBase {
     return MemoryEffectsBase(Location::InaccessibleMem, MR);
   }
 
-  /// Create MemoryEffectsBase that can only read inaccessible memory.
-  static MemoryEffectsBase
-  inaccessibleReadMemOnly(Location Loc = Location::InaccessibleMem) {
-    return MemoryEffectsBase(Loc, ModRefInfo::Ref);
-  }
-
-  /// Create MemoryEffectsBase that can only write inaccessible memory.
-  static MemoryEffectsBase
-  inaccessibleWriteMemOnly(Location Loc = Location::InaccessibleMem) {
-    return MemoryEffectsBase(Loc, ModRefInfo::Mod);
-  }
-
   /// Create MemoryEffectsBase that can only access errno memory.
   static MemoryEffectsBase errnoMemOnly(ModRefInfo MR = ModRefInfo::ModRef) {
     return MemoryEffectsBase(Location::ErrnoMem, MR);
diff --git a/llvm/lib/AsmParser/LLLexer.cpp b/llvm/lib/AsmParser/LLLexer.cpp
@@ -701,14 +701,12 @@ lltok::Kind LLLexer::LexIdentifier() {
   KEYWORD(write);
   KEYWORD(readwrite);
   KEYWORD(argmem);
-  KEYWORD(fpmr);
-  KEYWORD(za);
+  KEYWORD(aarch64_fpmr);
+  KEYWORD(aarch64_za);
   KEYWORD(inaccessiblemem);
   KEYWORD(errnomem);
   KEYWORD(argmemonly);
   KEYWORD(inaccessiblememonly);
-  KEYWORD(inaccessiblewritememonly);
-  KEYWORD(inaccessiblereadmemonly);
   KEYWORD(inaccessiblemem_or_argmemonly);
   KEYWORD(nocapture);
   KEYWORD(address_is_null);
diff --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp
@@ -1674,40 +1674,17 @@ static std::optional<MemoryEffects::Location> keywordToLoc(lltok::Kind Tok) {
     return IRMemLocation::InaccessibleMem;
   case lltok::kw_errnomem:
     return IRMemLocation::ErrnoMem;
-  case lltok::kw_fpmr:
+  case lltok::kw_aarch64_fpmr:
     return static_cast<IRMemLocation>(
         llvm::InaccessibleTargetMemLocation::AARCH64_FPMR);
-  case lltok::kw_za:
+  case lltok::kw_aarch64_za:
     return static_cast<IRMemLocation>(
         llvm::InaccessibleTargetMemLocation::AARCH64_ZA);
   default:
     return std::nullopt;
   }
 }
 
-bool LLParser::parseInaccessibleMemLocation(IRMemLocation &MemLoc) {
-  // It does not have location
-  if (Lex.getKind() != llvm::lltok::lparen)
-    return false;
-
-  Lex.Lex(); // eat '('
-
-  std::optional<IRMemLocation> LocOpt = keywordToLoc(Lex.getKind());
-  if (!LocOpt)
-    return tokError("invalid memory location keyword");
-
-  MemLoc = *LocOpt;
-
-  Lex.Lex(); // eat the keyword (e.g., 'fpmr', 'za')
-
-  if (Lex.getKind() != llvm::lltok::rparen)
-    return tokError("expected ')' after memory location");
-
-  Lex.Lex(); // eat ')'
-
-  return true; // success
-}
-
 /// parseFnAttributeValuePairs
 ///   ::= <attr> | <attr> '=' <value>
 bool LLParser::parseFnAttributeValuePairs(AttrBuilder &B,
@@ -1718,11 +1695,6 @@ bool LLParser::parseFnAttributeValuePairs(AttrBuilder &B,
   B.clear();
 
   MemoryEffects ME = MemoryEffects::unknown();
-  // Memory effects can span multiple locations, so we initialize a base
-  // MemoryEffects object once with default state, and then incrementally
-  // populate or combine effects for individual locations. This avoids
-  // rebuilding the full Data structure on each addition.
-  bool FirstME = true;
   while (true) {
     lltok::Kind Token = Lex.getKind();
     if (Token == lltok::rbrace)
@@ -1759,36 +1731,6 @@ bool LLParser::parseFnAttributeValuePairs(AttrBuilder &B,
       continue;
     }
 
-    if (Token == lltok::kw_inaccessiblereadmemonly) {
-      Lex.Lex(); // eat the keyword
-
-      IRMemLocation MemLoc;
-      if (parseInaccessibleMemLocation(MemLoc)) {
-        if (!FirstME)
-          ME = ME.getWithModRef(MemLoc, ModRefInfo::Ref);
-        else
-          ME &= MemoryEffects::inaccessibleReadMemOnly(MemLoc);
-      } else
-        ME = MemoryEffects::inaccessibleReadMemOnly();
-      FirstME = false;
-      continue;
-    }
-
-    if (Token == lltok::kw_inaccessiblewritememonly) {
-      Lex.Lex(); // eat the keyword
-
-      IRMemLocation MemLoc;
-      if (parseInaccessibleMemLocation(MemLoc)) {
-        if (!FirstME)
-          ME = ME.getWithModRef(MemLoc, ModRefInfo::Mod);
-        else
-          ME &= MemoryEffects::inaccessibleWriteMemOnly(MemLoc);
-      } else
-        ME &= MemoryEffects::inaccessibleWriteMemOnly();
-      FirstME = false;
-      continue;
-    }
-
     Attribute::AttrKind Attr = tokenToAttribute(Token);
     if (Attr == Attribute::None) {
       if (!InAttrGrp)
@@ -2597,10 +2539,6 @@ static std::optional<ModRefInfo> keywordToModRef(lltok::Kind Tok) {
     return ModRefInfo::Mod;
   case lltok::kw_readwrite:
     return ModRefInfo::ModRef;
-  case lltok::kw_inaccessiblewritememonly:
-    return ModRefInfo::Mod;
-  case lltok::kw_inaccessiblereadmemonly:
-    return ModRefInfo::Ref;
   default:
     return std::nullopt;
   }
diff --git a/llvm/lib/IR/Attributes.cpp b/llvm/lib/IR/Attributes.cpp
@@ -664,9 +664,9 @@ std::string Attribute::getAsString(bool InAttrGrp) const {
         InaccessibleTargetMemLocation TargetLoc =
             static_cast<InaccessibleTargetMemLocation>(Loc);
         if (TargetLoc == InaccessibleTargetMemLocation::AARCH64_FPMR)
-          OS << "fpmr: ";
+          OS << "aarch64_fpmr: ";
         if (TargetLoc == InaccessibleTargetMemLocation::AARCH64_ZA)
-          OS << "za: ";
+          OS << "aarch64_za: ";
         break;
       }
       }
diff --git a/llvm/lib/Support/ModRef.cpp b/llvm/lib/Support/ModRef.cpp
@@ -53,9 +53,9 @@ raw_ostream &llvm::operator<<(raw_ostream &OS, MemoryEffects ME) {
       InaccessibleTargetMemLocation TargetLoc =
           static_cast<InaccessibleTargetMemLocation>(Loc);
       if (TargetLoc == InaccessibleTargetMemLocation::AARCH64_FPMR)
-        OS << "FPMR: ";
+        OS << "AARCH64_FPMR: ";
       if (TargetLoc == InaccessibleTargetMemLocation::AARCH64_ZA)
-        OS << "ZA: ";
+        OS << "AARCH64_ZA: ";
       break;
     }
     }
diff --git a/llvm/lib/TableGen/Record.cpp b/llvm/lib/TableGen/Record.cpp
@@ -3108,7 +3108,7 @@ llvm::IRMemLocation Record::getLocationTypeAsInt(StringRef FieldName) const {
   if (Name == "AArch64_FPMR")
     return static_cast<IRMemLocation>(
         llvm::InaccessibleTargetMemLocation::AARCH64_FPMR);
-  else if (Name == "ZA")
+  else if (Name == "AArch64_ZA")
     return static_cast<IRMemLocation>(
         llvm::InaccessibleTargetMemLocation::AARCH64_ZA);
   else if (Name == "InaccessibleMem")
diff --git a/llvm/test/Assembler/memory-attribute.ll b/llvm/test/Assembler/memory-attribute.ll
@@ -79,23 +79,27 @@ declare void @fn_argmem_read_inaccessiblemem_write()
 declare void @fn_argmem_read_inaccessiblemem_write_reordered()
     memory(inaccessiblemem: write, argmem: read)
 
-; CHECK: Function Attrs: memory(za: write)
-; CHECK: @fn_argmem_read_inaccessiblemem_write_za()
-declare void @fn_argmem_read_inaccessiblemem_write_za()
-    memory(za: inaccessiblewritememonly)
-
-; CHECK: Function Attrs: memory(za: read)
-; CHECK: @fn_argmem_read_inaccessiblemem_read_za()
-declare void @fn_argmem_read_inaccessiblemem_read_za()
-    memory(za: inaccessiblereadmemonly)
-
-; CHECK: Function Attrs: memory(fpmr: write)
-; CHECK: @fn_argmem_read_inaccessiblemem_write_fpmr()
-declare void @fn_argmem_read_inaccessiblemem_write_fpmr()
-    memory(fpmr: inaccessiblewritememonly)
-
-; CHECK: Function Attrs: memory(fpmr: read)
-; CHECK: @fn_argmem_read_inaccessiblemem_read_fpmr()
-declare void @fn_argmem_read_inaccessiblemem_read_fpmr()
-    memory(fpmr: inaccessiblereadmemonly)
-
+; CHECK: Function Attrs: memory(aarch64_za: write)
+; CHECK: @fn_inaccessiblemem_write_aarch64_za()
+declare void @fn_inaccessiblemem_write_aarch64_za()
+    memory(aarch64_za: write)
+
+; CHECK: Function Attrs: memory(aarch64_za: read)
+; CHECK: @fn_inaccessiblemem_read_aarch64_za()
+declare void @fn_inaccessiblemem_read_aarch64_za()
+    memory(aarch64_za: read)
+
+; CHECK: Function Attrs: memory(aarch64_fpmr: write)
+; CHECK: @fn_inaccessiblemem_write_aarch64_fpmr()
+declare void @fn_inaccessiblemem_write_aarch64_fpmr()
+    memory(aarch64_fpmr: write)
+
+; CHECK: Function Attrs: memory(aarch64_fpmr: read)
+; CHECK: @fn_inaccessiblemem_read_aarch64_fpmr()
+declare void @fn_inaccessiblemem_read_aarch64_fpmr()
+    memory(aarch64_fpmr: read)
+
+; CHECK: Function Attrs: memory(aarch64_fpmr: read, aarch64_za: write)
+; CHECK: @fn_inaccessiblemem_read_aarch64_fpmr_write_aarch64_za()
+declare void @fn_inaccessiblemem_read_aarch64_fpmr_write_aarch64_za()
+    memory(aarch64_fpmr: read, aarch64_za: write)
diff --git a/llvm/test/Bitcode/attributes.ll b/llvm/test/Bitcode/attributes.ll
@@ -286,7 +286,6 @@ define void @f48() inaccessiblememonly {
 define void @f49() inaccessiblemem_or_argmemonly {
   ret void
 }
-
 ; CHECK: define void @f50(ptr swiftself %0)
 define void @f50(ptr swiftself %0)
 {
@@ -573,43 +572,6 @@ define void @dead_on_return(ptr dead_on_return %p) {
   ret void
 }
 
-; CHECK: define void @fpmr_write() [[ATTR59:#.*]] {
-define void @fpmr_write() inaccessiblewritememonly(fpmr) {
-  ret void
-}
-
-; CHECK: define void @fpmr_read()  [[ATTR60:#.*]] {
-define void @fpmr_read() inaccessiblereadmemonly(fpmr) {
-  ret void
-}
-
-; CHECK: define void @za_write()  [[ATTR61:#.*]] {
-define void @za_write() inaccessiblewritememonly(za) {
-  ret void
-}
-
-; CHECK: define void @za_read()  [[ATTR62:#.*]] {
-define void @za_read() inaccessiblereadmemonly(za) {
-  ret void
-}
-
-; CHECK: define void @read_inaccessible()  [[ATTR63:#.*]] {
-define void @read_inaccessible() inaccessiblereadmemonly {
-  ret void
-}
-
-; CHECK: define void @write_inaccessible()  [[ATTR64:#.*]] {
-define void @write_inaccessible() inaccessiblewritememonly {
-  ret void
-}
-
-; CHECK: define void @za_read_write_fpmr()  [[ATTR65:#.*]] {
-define void @za_read_write_fpmr() inaccessiblereadmemonly(za) inaccessiblewritememonly(fpmr) {
-  ret void
-}
-
-
-; CHECK: attributes #0 = { noreturn }
 ; CHECK: attributes #1 = { nounwind }
 ; CHECK: attributes #2 = { memory(none) }
 ; CHECK: attributes #3 = { memory(read) }
@@ -668,11 +630,4 @@ define void @za_read_write_fpmr() inaccessiblereadmemonly(za) inaccessiblewritem
 ; CHECK: attributes [[SKIPPROFILE]] = { skipprofile }
 ; CHECK: attributes [[OPTDEBUG]] = { optdebug }
 ; CHECK: attributes [[NODIVERGENCESOURCE]] = { nodivergencesource }
-; CHECK: attributes [[ATTR59]] = { memory(fpmr: write) }
-; CHECK: attributes [[ATTR60]] = { memory(fpmr: read) }
-; CHECK: attributes [[ATTR61]] = { memory(za: write) }
-; CHECK: attributes [[ATTR62]] = { memory(za: read) }
-; CHECK: attributes [[ATTR63]] = { memory(inaccessiblemem: read) }
-; CHECK: attributes [[ATTR64]] = { memory(inaccessiblemem: write) }
-; CHECK: attributes [[ATTR65]] = { memory(fpmr: write, za: read) }
 ; CHECK: attributes #[[NOBUILTIN]] = { nobuiltin }
diff --git a/llvm/test/TableGen/intrinsic-attrs-fp8.td b/llvm/test/TableGen/intrinsic-attrs-fp8.td
@@ -4,21 +4,51 @@ include "llvm/IR/Intrinsics.td"
 
 def int_aarch64_set_fpmr_2   : DefaultAttrsIntrinsic<[], [llvm_i64_ty], [IntrInaccessibleWriteMemOnly<AArch64_FPMR>]>;
 
-// CHECK:      static constexpr unsigned IntrinsicNameOffsetTable[] = {
-// CHECK-NEXT: 1, // not_intrinsic
-// CHECK-NEXT: 15, // llvm.aarch64.set.fpmr.2
+def int_aarch64_get_za_2   : DefaultAttrsIntrinsic<[], [llvm_i64_ty], [IntrInaccessibleReadMemOnly<AArch64_ZA>]>;
 
+def int_aarch64_get_fpmr_set_za   : DefaultAttrsIntrinsic<[], [llvm_i64_ty], [IntrInaccessibleReadMemOnly<AArch64_FPMR>, IntrInaccessibleWriteMemOnly<AArch64_ZA>]>;
 
-// CHECK: static AttributeSet getIntrinsicFnAttributeSet(LLVMContext &C, unsigned ID) {
-// CHECK-NEXT: switch (ID) {
-// CHECK-NEXT:  default: llvm_unreachable("Invalid attribute set number");
+// CHECK:    static constexpr unsigned IntrinsicNameOffsetTable[] = {
+// CHECK-NEXT:  1, // not_intrinsic
+// CHECK-NEXT:  15, // llvm.aarch64.get.fpmr.set.za
+// CHECK-NEXT:  44, // llvm.aarch64.get.za.2
+// CHECK-NEXT:  66, // llvm.aarch64.set.fpmr.2
+
+// CHECK:   static AttributeSet getIntrinsicFnAttributeSet(LLVMContext &C, unsigned ID) {
+// CHECK-NEXT:  switch (ID) {
+// CHECK-NEXT:    default: llvm_unreachable("Invalid attribute set number");
 // CHECK-NEXT:  case 0:
 // CHECK-NEXT:    return AttributeSet::get(C, {
 // CHECK-NEXT:      Attribute::get(C, Attribute::NoUnwind),
 // CHECK-NEXT:      Attribute::get(C, Attribute::NoCallback),
 // CHECK-NEXT:      Attribute::get(C, Attribute::NoSync),
 // CHECK-NEXT:      Attribute::get(C, Attribute::NoFree),
 // CHECK-NEXT:      Attribute::get(C, Attribute::WillReturn),
-// CHECK-NEXT:      // ArgMem: NoModRef, InaccessibleMem: NoModRef, ErrnoMem: NoModRef, FPMR: Mod, ZA: NoModRef, Other: NoModRef
-// CHECK-NEXT:      Attribute::getWithMemoryEffects(C, MemoryEffects::createFromIntValue(128)),
+// CHECK-NEXT:      // ArgMem: ModRef, InaccessibleMem: ModRef, ErrnoMem: ModRef, AARCH64_FPMR: Ref, AARCH64_ZA: Mod, Other: ModRef
+// CHECK-NEXT:      Attribute::getWithMemoryEffects(C, MemoryEffects::createFromIntValue(3711)),
+// CHECK-NEXT:    });
+// CHECK-NEXT:  case 1:
+// CHECK-NEXT:    return AttributeSet::get(C, {
+// CHECK-NEXT:      Attribute::get(C, Attribute::NoUnwind),
+// CHECK-NEXT:      Attribute::get(C, Attribute::NoCallback),
+// CHECK-NEXT:      Attribute::get(C, Attribute::NoSync),
+// CHECK-NEXT:      Attribute::get(C, Attribute::NoFree),
+// CHECK-NEXT:      Attribute::get(C, Attribute::WillReturn),
+// CHECK-NEXT:      // ArgMem: ModRef, InaccessibleMem: ModRef, ErrnoMem: ModRef, AARCH64_FPMR: ModRef, AARCH64_ZA: Ref, Other: ModRef
+// CHECK-NEXT:      Attribute::getWithMemoryEffects(C, MemoryEffects::createFromIntValue(3583)),
 // CHECK-NEXT:    });
+// CHECK-NEXT:  case 2:
+// CHECK-NEXT:    return AttributeSet::get(C, {
+// CHECK-NEXT:      Attribute::get(C, Attribute::NoUnwind),
+// CHECK-NEXT:      Attribute::get(C, Attribute::NoCallback),
+// CHECK-NEXT:      Attribute::get(C, Attribute::NoSync),
+// CHECK-NEXT:      Attribute::get(C, Attribute::NoFree),
+// CHECK-NEXT:      Attribute::get(C, Attribute::WillReturn),
+// CHECK-NEXT:      // ArgMem: ModRef, InaccessibleMem: ModRef, ErrnoMem: ModRef, AARCH64_FPMR: Mod, AARCH64_ZA: ModRef, Other: ModRef
+// CHECK-NEXT:      Attribute::getWithMemoryEffects(C, MemoryEffects::createFromIntValue(4031)),
+
+// CHECK: static constexpr uint16_t IntrinsicsToAttributesMap[] = {
+// CHECK-NEXT:    0 << 8 | 0, // llvm.aarch64.get.fpmr.set.za
+// CHECK-NEXT:    1 << 8 | 0, // llvm.aarch64.get.za.2
+// CHECK-NEXT:    2 << 8 | 0, // llvm.aarch64.set.fpmr.2
+// CHECK-NEXT:};
diff --git a/llvm/unittests/Support/ModRefTest.cpp b/llvm/unittests/Support/ModRefTest.cpp
@@ -21,7 +21,8 @@ TEST(ModRefTest, PrintMemoryEffects) {
   raw_string_ostream OS(S);
   OS << MemoryEffects::none();
   EXPECT_EQ(S, "ArgMem: NoModRef, InaccessibleMem: NoModRef, ErrnoMem: "
-               "NoModRef, FPMR: NoModRef, ZA: NoModRef, Other: NoModRef");
+               "NoModRef, AARCH64_FPMR: NoModRef, AARCH64_ZA: NoModRef, Other: "
+               "NoModRef");
 }
 
 } // namespace
diff --git a/llvm/utils/TableGen/Basic/CodeGenIntrinsics.cpp b/llvm/utils/TableGen/Basic/CodeGenIntrinsics.cpp
@@ -376,10 +376,10 @@ void CodeGenIntrinsic::setProperty(const Record *R) {
     ME &= MemoryEffects::inaccessibleMemOnly();
   else if (R->isSubClassOf("IntrInaccessibleReadMemOnly")) {
     llvm::IRMemLocation Loc = R->getLocationTypeAsInt("Loc");
-    ME &= MemoryEffects::inaccessibleWriteMemOnly(Loc);
+    ME = ME.getWithModRef(Loc, ModRefInfo::Ref);
   } else if (R->isSubClassOf("IntrInaccessibleWriteMemOnly")) {
     llvm::IRMemLocation Loc = R->getLocationTypeAsInt("Loc");
-    ME &= MemoryEffects::inaccessibleWriteMemOnly(Loc);
+    ME = ME.getWithModRef(Loc, ModRefInfo::Mod);
   } else if (R->getName() == "IntrInaccessibleMemOrArgMemOnly")
     ME &= MemoryEffects::inaccessibleOrArgMemOnly();
   else if (R->getName() == "Commutative")

Original file line number	Diff line number	Diff line change
`@@ -664,9 +664,9 @@ std::string Attribute::getAsString(bool InAttrGrp) const {`
`664`	`664`	`InaccessibleTargetMemLocation TargetLoc =`
`665`	`665`	`static_cast<InaccessibleTargetMemLocation>(Loc);`
`666`	`666`	`if (TargetLoc == InaccessibleTargetMemLocation::AARCH64_FPMR)`
`667`		`- OS << "fpmr: ";`
	`667`	`+ OS << "aarch64_fpmr: ";`
`668`	`668`	`if (TargetLoc == InaccessibleTargetMemLocation::AARCH64_ZA)`
`669`		`- OS << "za: ";`
	`669`	`+ OS << "aarch64_za: ";`
`670`	`670`	`break;`
`671`	`671`	`}`
`672`	`672`	`}`
Original file line number	Diff line number	Diff line change
`@@ -53,9 +53,9 @@ raw_ostream &llvm::operator<<(raw_ostream &OS, MemoryEffects ME) {`
`53`	`53`	`InaccessibleTargetMemLocation TargetLoc =`
`54`	`54`	`static_cast<InaccessibleTargetMemLocation>(Loc);`
`55`	`55`	`if (TargetLoc == InaccessibleTargetMemLocation::AARCH64_FPMR)`
`56`		`- OS << "FPMR: ";`
	`56`	`+ OS << "AARCH64_FPMR: ";`
`57`	`57`	`if (TargetLoc == InaccessibleTargetMemLocation::AARCH64_ZA)`
`58`		`- OS << "ZA: ";`
	`58`	`+ OS << "AARCH64_ZA: ";`
`59`	`59`	`break;`
`60`	`60`	`}`
`61`	`61`	`}`