Skip to content

[RFC] Extend MemoryEffects to Support Target-Specific Memory Locations #148650

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions llvm/include/llvm/AsmParser/LLToken.h
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,8 @@ enum Kind {
kw_readwrite,
kw_argmem,
kw_inaccessiblemem,
kw_aarch64_fpmr,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll let @nikic comment, but are we really planning to extend LLVM IR syntax with arch specific keywords?

Copy link
Collaborator

@paulwalker-arm paulwalker-arm Aug 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this new? There are many examples mainly linked to calling conventions and function/argument attributes.

kw_aarch64_za,
kw_errnomem,

// Legacy attributes:
Expand Down
11 changes: 11 additions & 0 deletions llvm/include/llvm/IR/Intrinsics.td
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,17 @@ def IntrArgMemOnly : IntrinsicProperty;
// accessible by the module being compiled. This is a weaker form of IntrNoMem.
def IntrInaccessibleMemOnly : IntrinsicProperty;



class IntrinsicMemoryLocation;
// This should be added in the Target, but once in IntrinsicsAArch64.td
// It complains error: "Variable not defined: 'AArch64_FPMR'"
def AArch64_FPMR : IntrinsicMemoryLocation;
def AArch64_ZA: IntrinsicMemoryLocation;
// IntrInaccessible{Read|Write}MemOnly needs to set Location
class IntrInaccessibleReadMemOnly<IntrinsicMemoryLocation idx> : IntrinsicProperty{IntrinsicMemoryLocation Loc=idx;}
class IntrInaccessibleWriteMemOnly<IntrinsicMemoryLocation idx> : IntrinsicProperty{IntrinsicMemoryLocation Loc=idx;}

// IntrInaccessibleMemOrArgMemOnly -- This intrinsic only accesses memory that
// its pointer-typed arguments point to or memory that is not accessible
// by the module being compiled. This is a weaker form of IntrArgMemOnly.
Expand Down
46 changes: 45 additions & 1 deletion llvm/include/llvm/Support/ModRef.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,11 @@ enum class ModRefInfo : uint8_t {
/// Debug print ModRefInfo.
LLVM_ABI raw_ostream &operator<<(raw_ostream &OS, ModRefInfo MR);

enum class InaccessibleTargetMemLocation {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Each Target should have an InaccessibleTargetMemLocation, but I am not sure if we should check which Target it is being used in Support, Attributes and so on. I would appreciate some suggestion if this is the right way to do this.

And this is still using Data to model the Memory Locations with ModRef. Is this the only way to add new memory location?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we could have something around the following instead:

enum class IRMemLocation {
  ArgMem = 0,
  InaccessibleMem = 1,
  ErrnoMem = 2,
  TargetSpecificMem = 3,
  Other = 4,

  First = ArgMem,
  Last = Other,
};

And then extending MemoryEffectsBase to have, like, a Optional<TargetSpecificMemTag> TargetMem; field, where TargetMem is, e.g.,:

struct TargetSpecificMemTag {
  Triple::ArchType Arch;
  uint8_t Kind;
};

This would lead to know easily if the location is target-specific:

bool isTargetSpecific() const { return Loc == Location::TargetSpecificMem; }

And I guess it would avoid having some static_cast<IRMemLocation> around. Not sure if the target-specific memory effects are decoupled enough this way though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @antoniofrighetto
I will try to follow your suggestion in a next iteration. I am not sure I fully understand the consequences of implementing this way.

For instance: I am not sure I can have only one Kind.

For instance, something like this:
def int_aarch64_get_fpmr_set_za : DefaultAttrsIntrinsic<[], [llvm_i64_ty], [IntrInaccessibleReadMemOnly<AArch64_FPMR>, IntrInaccessibleWriteMemOnly<AArch64_ZA>]>;

I imagine that the ModRefInfo for TargetSpecificMem will be 11, that is ModRef.
But how Kind will work in the case above? Do you have an idea? Because we need to set one to just write and another to just read.
I can only think about another TargetMemoryLocation and another vector like Data, but only used when TargetSpecificMem !=NoModRef

I would also have to see how this will work in BasicAliasAnalysis.cpp/getModRefInfo and BitcodeReader.cpp(parseAttributeGroupBlock)

AARCH64_FPMR = 3,
AARCH64_ZA = 4,
};

/// The locations at which a function might access memory.
enum class IRMemLocation {
/// Access to memory via argument pointers.
Expand All @@ -65,7 +70,7 @@ enum class IRMemLocation {
/// Errno memory.
ErrnoMem = 2,
/// Any other memory.
Other = 3,
Other = 5,

/// Helpers to iterate all locations in the MemoryEffectsBase class.
First = ArgMem,
Expand Down Expand Up @@ -152,6 +157,40 @@ template <typename LocationEnum> class MemoryEffectsBase {
return MemoryEffectsBase(Location::Other, 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);
}

/// Checks if only target-specific memory locations are set.
/// Ignores standard locations like ArgMem or InaccessibleMem.
/// Needed because `Data` may be non-zero by default unless explicitly
/// cleared.
bool onlyAccessTargetMemoryLocation() {
MemoryEffectsBase ME = *this;
for (unsigned I = static_cast<int>(LocationEnum::ErrnoMem);
I < static_cast<int>(LocationEnum::Last); I++)
ME = ME.getWithoutLoc(static_cast<IRMemLocation>(I));
return ME.doesNotAccessMemory();
}

/// Create MemoryEffectsBase that can only access Target Memory Locations
static MemoryEffectsBase
setTargetMemLocationModRef(ModRefInfo MR = ModRefInfo::NoModRef) {
MemoryEffectsBase FRMB = none();
for (unsigned I = static_cast<int>(LocationEnum::ErrnoMem);
I < static_cast<int>(LocationEnum::Last); I++)
FRMB.setModRef(static_cast<Location>(I), MR);
return FRMB;
}

/// Create MemoryEffectsBase that can only access inaccessible or argument
/// memory.
static MemoryEffectsBase
Expand All @@ -178,6 +217,11 @@ template <typename LocationEnum> class MemoryEffectsBase {
return MemoryEffectsBase(Data);
}

bool isTargetMemLoc(IRMemLocation Loc) {
return static_cast<unsigned>(Loc) >
static_cast<unsigned>(Location::ErrnoMem);
}

/// Convert MemoryEffectsBase into an encoded integer value (used by memory
/// attribute).
uint32_t toIntValue() const {
Expand Down
3 changes: 3 additions & 0 deletions llvm/include/llvm/TableGen/Record.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Casting.h"
#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/ModRef.h"
#include "llvm/Support/SMLoc.h"
#include "llvm/Support/Timer.h"
#include "llvm/Support/TrailingObjects.h"
Expand Down Expand Up @@ -1961,6 +1962,8 @@ class Record {
/// value is not the right type.
int64_t getValueAsInt(StringRef FieldName) const;

llvm::IRMemLocation getLocationTypeAsInt(StringRef FieldName) const;

/// This method looks up the specified field and returns its value as an Dag,
/// throwing an exception if the field does not exist or if the value is not
/// the right type.
Expand Down
2 changes: 2 additions & 0 deletions llvm/lib/AsmParser/LLLexer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -701,6 +701,8 @@ lltok::Kind LLLexer::LexIdentifier() {
KEYWORD(write);
KEYWORD(readwrite);
KEYWORD(argmem);
KEYWORD(aarch64_fpmr);
KEYWORD(aarch64_za);
KEYWORD(inaccessiblemem);
KEYWORD(errnomem);
KEYWORD(argmemonly);
Expand Down
32 changes: 19 additions & 13 deletions llvm/lib/AsmParser/LLParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1666,6 +1666,25 @@ static bool upgradeMemoryAttr(MemoryEffects &ME, lltok::Kind Kind) {
}
}

static std::optional<MemoryEffects::Location> keywordToLoc(lltok::Kind Tok) {
switch (Tok) {
case lltok::kw_argmem:
return IRMemLocation::ArgMem;
case lltok::kw_inaccessiblemem:
return IRMemLocation::InaccessibleMem;
case lltok::kw_errnomem:
return IRMemLocation::ErrnoMem;
case lltok::kw_aarch64_fpmr:
return static_cast<IRMemLocation>(
llvm::InaccessibleTargetMemLocation::AARCH64_FPMR);
case lltok::kw_aarch64_za:
return static_cast<IRMemLocation>(
llvm::InaccessibleTargetMemLocation::AARCH64_ZA);
default:
return std::nullopt;
}
}

/// parseFnAttributeValuePairs
/// ::= <attr> | <attr> '=' <value>
bool LLParser::parseFnAttributeValuePairs(AttrBuilder &B,
Expand Down Expand Up @@ -2510,19 +2529,6 @@ bool LLParser::parseAllocKind(AllocFnKind &Kind) {
return false;
}

static std::optional<MemoryEffects::Location> keywordToLoc(lltok::Kind Tok) {
switch (Tok) {
case lltok::kw_argmem:
return IRMemLocation::ArgMem;
case lltok::kw_inaccessiblemem:
return IRMemLocation::InaccessibleMem;
case lltok::kw_errnomem:
return IRMemLocation::ErrnoMem;
default:
return std::nullopt;
}
}

static std::optional<ModRefInfo> keywordToModRef(lltok::Kind Tok) {
switch (Tok) {
case lltok::kw_none:
Expand Down
13 changes: 13 additions & 0 deletions llvm/lib/IR/Attributes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -640,6 +640,10 @@ std::string Attribute::getAsString(bool InAttrGrp) const {
if (MR == OtherMR)
continue;

// Dont want to print Target Location if NoModRef
if (ME.isTargetMemLoc(Loc) && (MR == ModRefInfo::NoModRef))
continue;

if (!First)
OS << ", ";
First = false;
Expand All @@ -656,6 +660,15 @@ std::string Attribute::getAsString(bool InAttrGrp) const {
break;
case IRMemLocation::Other:
llvm_unreachable("This is represented as the default access kind");
default: {
InaccessibleTargetMemLocation TargetLoc =
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How can we make this to not be target specific? Any suggestion?

static_cast<InaccessibleTargetMemLocation>(Loc);
if (TargetLoc == InaccessibleTargetMemLocation::AARCH64_FPMR)
OS << "aarch64_fpmr: ";
if (TargetLoc == InaccessibleTargetMemLocation::AARCH64_ZA)
OS << "aarch64_za: ";
break;
}
}
OS << getModRefStr(MR);
}
Expand Down
9 changes: 9 additions & 0 deletions llvm/lib/Support/ModRef.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,15 @@ raw_ostream &llvm::operator<<(raw_ostream &OS, MemoryEffects ME) {
case IRMemLocation::Other:
OS << "Other: ";
break;
default: {
InaccessibleTargetMemLocation TargetLoc =
static_cast<InaccessibleTargetMemLocation>(Loc);
if (TargetLoc == InaccessibleTargetMemLocation::AARCH64_FPMR)
OS << "AARCH64_FPMR: ";
if (TargetLoc == InaccessibleTargetMemLocation::AARCH64_ZA)
OS << "AARCH64_ZA: ";
break;
}
}
OS << ME.getModRef(Loc);
});
Expand Down
15 changes: 15 additions & 0 deletions llvm/lib/TableGen/Record.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3102,6 +3102,21 @@ Record::getValueAsListOfDefs(StringRef FieldName) const {
return Defs;
}

llvm::IRMemLocation Record::getLocationTypeAsInt(StringRef FieldName) const {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function does not belong here. It should be moved instead to the specific backend that uses it (intrinsic emitter I am assuming)

const Record *LocRec = getValueAsDef(FieldName);
StringRef Name = LocRec->getName();
if (Name == "AArch64_FPMR")
return static_cast<IRMemLocation>(
llvm::InaccessibleTargetMemLocation::AARCH64_FPMR);
else if (Name == "AArch64_ZA")
return static_cast<IRMemLocation>(
llvm::InaccessibleTargetMemLocation::AARCH64_ZA);
else if (Name == "InaccessibleMem")
return llvm::IRMemLocation::InaccessibleMem;
else
PrintFatalError(getLoc(), "unknown IRMemLocation: " + Name);
}

int64_t Record::getValueAsInt(StringRef FieldName) const {
const RecordVal *R = getValue(FieldName);
if (!R || !R->getValue())
Expand Down
3 changes: 3 additions & 0 deletions llvm/lib/Transforms/IPO/FunctionAttrs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,9 @@ static void addLocAccess(MemoryEffects &ME, const MemoryLocation &Loc,
ME |= MemoryEffects::argMemOnly(MR);
ME |= MemoryEffects(IRMemLocation::ErrnoMem, MR);
ME |= MemoryEffects(IRMemLocation::Other, MR);
// Should also set the other Target Memory Locations as MR.
// To compares with MemoryEffects::unknown() in addMemoryAttrs
ME |= MemoryEffects::setTargetMemLocationModRef(MR);
}

static void addArgLocs(MemoryEffects &ME, const CallBase *Call,
Expand Down
25 changes: 25 additions & 0 deletions llvm/test/Assembler/memory-attribute.ll
Original file line number Diff line number Diff line change
Expand Up @@ -78,3 +78,28 @@ declare void @fn_argmem_read_inaccessiblemem_write()
; CHECK: @fn_argmem_read_inaccessiblemem_write_reordered()
declare void @fn_argmem_read_inaccessiblemem_write_reordered()
memory(inaccessiblemem: write, argmem: read)

; 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)
1 change: 0 additions & 1 deletion llvm/test/Bitcode/attributes.ll
Original file line number Diff line number Diff line change
Expand Up @@ -572,7 +572,6 @@ define void @dead_on_return(ptr dead_on_return %p) {
ret void
}

; CHECK: attributes #0 = { noreturn }
; CHECK: attributes #1 = { nounwind }
; CHECK: attributes #2 = { memory(none) }
; CHECK: attributes #3 = { memory(read) }
Expand Down
54 changes: 54 additions & 0 deletions llvm/test/TableGen/intrinsic-attrs-fp8.td
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
// RUN: llvm-tblgen -gen-intrinsic-impl -I %p/../../include -DTEST_INTRINSICS_SUPPRESS_DEFS %s | FileCheck %s

include "llvm/IR/Intrinsics.td"

def int_aarch64_set_fpmr_2 : DefaultAttrsIntrinsic<[], [llvm_i64_ty], [IntrInaccessibleWriteMemOnly<AArch64_FPMR>]>;

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 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, AARCH64_FPMR: Ref, AARCH64_ZA: Mod, Other: NoModRef
// CHECK-NEXT: Attribute::getWithMemoryEffects(C, MemoryEffects::createFromIntValue(576)),
// 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: NoModRef, InaccessibleMem: NoModRef, ErrnoMem: NoModRef, AARCH64_FPMR: NoModRef, AARCH64_ZA: Ref, Other: NoModRef
// CHECK-NEXT: Attribute::getWithMemoryEffects(C, MemoryEffects::createFromIntValue(256)),
// 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: NoModRef, InaccessibleMem: NoModRef, ErrnoMem: NoModRef, AARCH64_FPMR: Mod, AARCH64_ZA: NoModRef, Other: NoModRef
// CHECK-NEXT: Attribute::getWithMemoryEffects(C, MemoryEffects::createFromIntValue(128)),

// 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:};
3 changes: 2 additions & 1 deletion llvm/unittests/Support/ModRefTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ TEST(ModRefTest, PrintMemoryEffects) {
raw_string_ostream OS(S);
OS << MemoryEffects::none();
EXPECT_EQ(S, "ArgMem: NoModRef, InaccessibleMem: NoModRef, ErrnoMem: "
"NoModRef, Other: NoModRef");
"NoModRef, AARCH64_FPMR: NoModRef, AARCH64_ZA: NoModRef, Other: "
"NoModRef");
}

} // namespace
14 changes: 13 additions & 1 deletion llvm/utils/TableGen/Basic/CodeGenIntrinsics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,19 @@ void CodeGenIntrinsic::setProperty(const Record *R) {
ME &= MemoryEffects::argMemOnly();
else if (R->getName() == "IntrInaccessibleMemOnly")
ME &= MemoryEffects::inaccessibleMemOnly();
else if (R->getName() == "IntrInaccessibleMemOrArgMemOnly")
else if (R->isSubClassOf("IntrInaccessibleReadMemOnly")) {
llvm::IRMemLocation Loc = R->getLocationTypeAsInt("Loc");
if (ME.onlyAccessTargetMemoryLocation())
ME = ME.getWithModRef(Loc, ModRefInfo::Ref);
else
ME &= MemoryEffects::inaccessibleReadMemOnly(Loc);
} else if (R->isSubClassOf("IntrInaccessibleWriteMemOnly")) {
llvm::IRMemLocation Loc = R->getLocationTypeAsInt("Loc");
if (ME.onlyAccessTargetMemoryLocation())
ME = ME.getWithModRef(Loc, ModRefInfo::Mod);
else
ME &= MemoryEffects::inaccessibleWriteMemOnly(Loc);
} else if (R->getName() == "IntrInaccessibleMemOrArgMemOnly")
ME &= MemoryEffects::inaccessibleOrArgMemOnly();
else if (R->getName() == "Commutative")
isCommutative = true;
Expand Down
Loading