Skip to content

Commit 189d9f7

Browse files
CarolineConcattoCarolineConcatto
committed
Address review comments
1 parent 4103b30 commit 189d9f7

File tree

12 files changed

+66
-117
lines changed

12 files changed

+66
-117
lines changed

‎llvm/include/llvm/Support/ModRef.h‎

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -56,11 +56,6 @@ enum class ModRefInfo : uint8_t {
5656
/// Debug print ModRefInfo.
5757
LLVM_ABI raw_ostream &operator<<(raw_ostream &OS, ModRefInfo MR);
5858

59-
enum class InaccessibleTargetMemLocation {
60-
TargetMem0 = 3,
61-
TargetMem1 = 4,
62-
};
63-
6459
/// The locations at which a function might access memory.
6560
enum class IRMemLocation {
6661
/// Access to memory via argument pointers.
@@ -70,11 +65,16 @@ enum class IRMemLocation {
7065
/// Errno memory.
7166
ErrnoMem = 2,
7267
/// Any other memory.
73-
Other = 5,
68+
Other = 3,
69+
/// Target Memory Location
70+
/// Used by a target to represent target specif memory regions
71+
TargetMem0 = 4,
72+
TargetMem1 = 5,
7473

7574
/// Helpers to iterate all locations in the MemoryEffectsBase class.
7675
First = ArgMem,
77-
Last = Other,
76+
Last = TargetMem1,
77+
FirstTarget = TargetMem0,
7878
};
7979

8080
template <typename LocationEnum> class MemoryEffectsBase {
@@ -175,8 +175,8 @@ template <typename LocationEnum> class MemoryEffectsBase {
175175
/// cleared.
176176
bool onlyAccessTargetMemoryLocation() {
177177
MemoryEffectsBase ME = *this;
178-
for (unsigned I = static_cast<int>(LocationEnum::ErrnoMem);
179-
I < static_cast<int>(LocationEnum::Last); I++)
178+
for (unsigned I = static_cast<int>(LocationEnum::FirstTarget);
179+
I <= static_cast<int>(LocationEnum::Last); I++)
180180
ME = ME.getWithoutLoc(static_cast<IRMemLocation>(I));
181181
return ME.doesNotAccessMemory();
182182
}
@@ -185,7 +185,7 @@ template <typename LocationEnum> class MemoryEffectsBase {
185185
static MemoryEffectsBase
186186
setTargetMemLocationModRef(ModRefInfo MR = ModRefInfo::NoModRef) {
187187
MemoryEffectsBase FRMB = none();
188-
for (unsigned I = static_cast<int>(LocationEnum::ErrnoMem);
188+
for (unsigned I = static_cast<int>(LocationEnum::FirstTarget);
189189
I < static_cast<int>(LocationEnum::Last); I++)
190190
FRMB.setModRef(static_cast<Location>(I), MR);
191191
return FRMB;
@@ -218,8 +218,8 @@ template <typename LocationEnum> class MemoryEffectsBase {
218218
}
219219

220220
bool isTargetMemLoc(IRMemLocation Loc) {
221-
return static_cast<unsigned>(Loc) >
222-
static_cast<unsigned>(Location::ErrnoMem);
221+
return static_cast<unsigned>(Loc) >=
222+
static_cast<unsigned>(Location::FirstTarget);
223223
}
224224

225225
/// Convert MemoryEffectsBase into an encoded integer value (used by memory
@@ -276,10 +276,8 @@ template <typename LocationEnum> class MemoryEffectsBase {
276276

277277
/// Whether this function only (at most) accesses inaccessible memory.
278278
bool onlyAccessesInaccessibleMem() const {
279-
return getWithoutLoc(static_cast<IRMemLocation>(
280-
llvm::InaccessibleTargetMemLocation::TargetMem0))
281-
.getWithoutLoc(static_cast<IRMemLocation>(
282-
llvm::InaccessibleTargetMemLocation::TargetMem1))
279+
return getWithoutLoc(IRMemLocation::TargetMem0)
280+
.getWithoutLoc(IRMemLocation::TargetMem1)
283281
.getWithoutLoc(Location::InaccessibleMem)
284282
.doesNotAccessMemory();
285283
}

‎llvm/include/llvm/TableGen/Record.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
#include "llvm/ADT/StringRef.h"
2626
#include "llvm/Support/Casting.h"
2727
#include "llvm/Support/ErrorHandling.h"
28-
#include "llvm/Support/ModRef.h"
2928
#include "llvm/Support/SMLoc.h"
3029
#include "llvm/Support/Timer.h"
3130
#include "llvm/Support/TrailingObjects.h"

‎llvm/lib/AsmParser/LLParser.cpp‎

Lines changed: 17 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1666,25 +1666,6 @@ static bool upgradeMemoryAttr(MemoryEffects &ME, lltok::Kind Kind) {
16661666
}
16671667
}
16681668

1669-
static std::optional<MemoryEffects::Location> keywordToLoc(lltok::Kind Tok) {
1670-
switch (Tok) {
1671-
case lltok::kw_argmem:
1672-
return IRMemLocation::ArgMem;
1673-
case lltok::kw_inaccessiblemem:
1674-
return IRMemLocation::InaccessibleMem;
1675-
case lltok::kw_errnomem:
1676-
return IRMemLocation::ErrnoMem;
1677-
case lltok::kw_target_mem0:
1678-
return static_cast<IRMemLocation>(
1679-
llvm::InaccessibleTargetMemLocation::TargetMem0);
1680-
case lltok::kw_target_mem1:
1681-
return static_cast<IRMemLocation>(
1682-
llvm::InaccessibleTargetMemLocation::TargetMem1);
1683-
default:
1684-
return std::nullopt;
1685-
}
1686-
}
1687-
16881669
/// parseFnAttributeValuePairs
16891670
/// ::= <attr> | <attr> '=' <value>
16901671
bool LLParser::parseFnAttributeValuePairs(AttrBuilder &B,
@@ -2532,6 +2513,23 @@ bool LLParser::parseAllocKind(AllocFnKind &Kind) {
25322513
return false;
25332514
}
25342515

2516+
static std::optional<MemoryEffects::Location> keywordToLoc(lltok::Kind Tok) {
2517+
switch (Tok) {
2518+
case lltok::kw_argmem:
2519+
return IRMemLocation::ArgMem;
2520+
case lltok::kw_inaccessiblemem:
2521+
return IRMemLocation::InaccessibleMem;
2522+
case lltok::kw_errnomem:
2523+
return IRMemLocation::ErrnoMem;
2524+
case lltok::kw_target_mem0:
2525+
return IRMemLocation::TargetMem0;
2526+
case lltok::kw_target_mem1:
2527+
return IRMemLocation::TargetMem1;
2528+
default:
2529+
return std::nullopt;
2530+
}
2531+
}
2532+
25352533
static std::optional<ModRefInfo> keywordToModRef(lltok::Kind Tok) {
25362534
switch (Tok) {
25372535
case lltok::kw_none:

‎llvm/lib/IR/AsmWriter.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5011,8 +5011,8 @@ void AssemblyWriter::writeAllAttributeGroups() {
50115011
asVec[I.second] = I;
50125012

50135013
for (const auto &I : asVec)
5014-
Out << "attributes #" << I.second << " = { " << I.first.getAsString(true)
5015-
<< " }\n";
5014+
Out << "attributes #" << I.second << " = { "
5015+
<< I.first.getAsString(true) << " }\n";
50165016
}
50175017

50185018
void AssemblyWriter::printUseListOrder(const Value *V,

‎llvm/lib/IR/Attributes.cpp‎

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -661,20 +661,12 @@ std::string Attribute::getAsString(bool InAttrGrp) const {
661661
case IRMemLocation::Other:
662662
llvm_unreachable("This is represented as the default access kind");
663663
break;
664-
}
665-
// Target Memory locations are not IRMemLocation.
666-
// It is breaking buildbots when it treats warning as error
667-
if (static_cast<int>(Loc) > static_cast<int>(IRMemLocation::ErrnoMem)) {
668-
InaccessibleTargetMemLocation TargetMemLoc =
669-
static_cast<InaccessibleTargetMemLocation>(Loc);
670-
switch (TargetMemLoc) {
671-
case InaccessibleTargetMemLocation::TargetMem0:
672-
OS << "target_mem0: ";
673-
break;
674-
case InaccessibleTargetMemLocation::TargetMem1:
675-
OS << "target_mem1: ";
676-
break;
677-
}
664+
case IRMemLocation::TargetMem0:
665+
OS << "target_mem0: ";
666+
break;
667+
case IRMemLocation::TargetMem1:
668+
OS << "target_mem1: ";
669+
break;
678670
}
679671

680672
OS << getModRefStr(MR);

‎llvm/lib/Support/ModRef.cpp‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,12 +49,10 @@ raw_ostream &llvm::operator<<(raw_ostream &OS, MemoryEffects ME) {
4949
case IRMemLocation::Other:
5050
OS << "Other: ";
5151
break;
52-
case static_cast<IRMemLocation>(
53-
static_cast<int>(InaccessibleTargetMemLocation::TargetMem0)):
52+
case IRMemLocation::TargetMem0:
5453
OS << "TargetMem0: ";
5554
break;
56-
case static_cast<IRMemLocation>(
57-
static_cast<int>(InaccessibleTargetMemLocation::TargetMem1)):
55+
case IRMemLocation::TargetMem1:
5856
OS << "TargetMem1: ";
5957
break;
6058
}

‎llvm/lib/Transforms/IPO/FunctionAttrs.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,8 @@ static void addLocAccess(MemoryEffects &ME, const MemoryLocation &Loc,
144144
ME |= MemoryEffects(IRMemLocation::Other, MR);
145145
// Should also set the other Target Memory Locations as MR.
146146
// To compares with MemoryEffects::unknown() in addMemoryAttrs
147-
ME |= MemoryEffects::setTargetMemLocationModRef(MR);
147+
ME |= MemoryEffects(IRMemLocation::TargetMem0, MR);
148+
ME |= MemoryEffects(IRMemLocation::TargetMem1, MR);;
148149
}
149150

150151
static void addArgLocs(MemoryEffects &ME, const CallBase *Call,

‎llvm/test/Assembler/aarch64-memory-attribute.ll‎

Lines changed: 0 additions & 35 deletions
This file was deleted.

‎llvm/test/Assembler/memory-attribute.ll‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -79,26 +79,26 @@ declare void @fn_argmem_read_inaccessiblemem_write()
7979
declare void @fn_argmem_read_inaccessiblemem_write_reordered()
8080
memory(inaccessiblemem: write, argmem: read)
8181

82-
; CHECK: Function Attrs: memory(target_mem1: write)
83-
; CHECK: @fn_inaccessiblemem_write_mem_target1()
84-
declare void @fn_inaccessiblemem_write_mem_target1()
85-
memory(target_mem1: write)
86-
87-
; CHECK: Function Attrs: memory(target_mem1: read)
88-
; CHECK: @fn_inaccessiblemem_read_mem_target1()
89-
declare void @fn_inaccessiblemem_read_mem_target1()
90-
memory(target_mem1: read)
91-
9282
; CHECK: Function Attrs: memory(target_mem0: write)
93-
; CHECK: @fn_inaccessiblemem_write_target_mem0()
94-
declare void @fn_inaccessiblemem_write_target_mem0()
83+
; CHECK: @fn_inaccessiblemem_write_mem_target0()
84+
declare void @fn_inaccessiblemem_write_mem_target0()
9585
memory(target_mem0: write)
9686

9787
; CHECK: Function Attrs: memory(target_mem0: read)
98-
; CHECK: @fn_inaccessiblemem_read_target_mem0()
99-
declare void @fn_inaccessiblemem_read_target_mem0()
88+
; CHECK: @fn_inaccessiblemem_read_mem_target0()
89+
declare void @fn_inaccessiblemem_read_mem_target0()
10090
memory(target_mem0: read)
10191

92+
; CHECK: Function Attrs: memory(target_mem1: write)
93+
; CHECK: @fn_inaccessiblemem_write_target_mem1()
94+
declare void @fn_inaccessiblemem_write_target_mem1()
95+
memory(target_mem1: write)
96+
97+
; CHECK: Function Attrs: memory(target_mem1: read)
98+
; CHECK: @fn_inaccessiblemem_read_target_mem1()
99+
declare void @fn_inaccessiblemem_read_target_mem1()
100+
memory(target_mem1: read)
101+
102102
; CHECK: Function Attrs: memory(target_mem0: read, target_mem1: write)
103103
; CHECK: @fn_inaccessiblemem_read_target_mem0_write_mem_target1()
104104
declare void @fn_inaccessiblemem_read_target_mem0_write_mem_target1()

‎llvm/test/TableGen/intrinsic-attrs-fp8.td‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@ def int_aarch64_get_fpmr_set_za : DefaultAttrsIntrinsic<[], [llvm_i64_ty], [In
2424
// CHECK-NEXT: Attribute::get(C, Attribute::NoSync),
2525
// CHECK-NEXT: Attribute::get(C, Attribute::NoFree),
2626
// CHECK-NEXT: Attribute::get(C, Attribute::WillReturn),
27-
// CHECK-NEXT: // ArgMem: NoModRef, InaccessibleMem: NoModRef, ErrnoMem: NoModRef, TargetMem0: Ref, TargetMem1: Mod, Other: NoModRef
28-
// CHECK-NEXT: Attribute::getWithMemoryEffects(C, MemoryEffects::createFromIntValue(576)),
27+
// CHECK-NEXT: // ArgMem: NoModRef, InaccessibleMem: NoModRef, ErrnoMem: NoModRef, Other: NoModRef, TargetMem0: Ref, TargetMem1: Mod
28+
// CHECK-NEXT: Attribute::getWithMemoryEffects(C, MemoryEffects::createFromIntValue(2304)),
2929
// CHECK-NEXT: });
3030
// CHECK-NEXT: case 1:
3131
// CHECK-NEXT: return AttributeSet::get(C, {
@@ -34,8 +34,8 @@ def int_aarch64_get_fpmr_set_za : DefaultAttrsIntrinsic<[], [llvm_i64_ty], [In
3434
// CHECK-NEXT: Attribute::get(C, Attribute::NoSync),
3535
// CHECK-NEXT: Attribute::get(C, Attribute::NoFree),
3636
// CHECK-NEXT: Attribute::get(C, Attribute::WillReturn),
37-
// CHECK-NEXT: // ArgMem: NoModRef, InaccessibleMem: NoModRef, ErrnoMem: NoModRef, TargetMem0: NoModRef, TargetMem1: Ref, Other: NoModRef
38-
// CHECK-NEXT: Attribute::getWithMemoryEffects(C, MemoryEffects::createFromIntValue(256)),
37+
// CHECK-NEXT: // ArgMem: NoModRef, InaccessibleMem: NoModRef, ErrnoMem: NoModRef, Other: NoModRef, TargetMem0: NoModRef, TargetMem1: Ref
38+
// CHECK-NEXT: Attribute::getWithMemoryEffects(C, MemoryEffects::createFromIntValue(1024)),
3939
// CHECK-NEXT: });
4040
// CHECK-NEXT: case 2:
4141
// CHECK-NEXT: return AttributeSet::get(C, {
@@ -44,8 +44,8 @@ def int_aarch64_get_fpmr_set_za : DefaultAttrsIntrinsic<[], [llvm_i64_ty], [In
4444
// CHECK-NEXT: Attribute::get(C, Attribute::NoSync),
4545
// CHECK-NEXT: Attribute::get(C, Attribute::NoFree),
4646
// CHECK-NEXT: Attribute::get(C, Attribute::WillReturn),
47-
// CHECK-NEXT: // ArgMem: NoModRef, InaccessibleMem: NoModRef, ErrnoMem: NoModRef, TargetMem0: Mod, TargetMem1: NoModRef, Other: NoModRef
48-
// CHECK-NEXT: Attribute::getWithMemoryEffects(C, MemoryEffects::createFromIntValue(128)),
47+
// CHECK-NEXT: // ArgMem: NoModRef, InaccessibleMem: NoModRef, ErrnoMem: NoModRef, Other: NoModRef, TargetMem0: Mod, TargetMem1: NoModRef
48+
// CHECK-NEXT: Attribute::getWithMemoryEffects(C, MemoryEffects::createFromIntValue(512)),
4949

5050
// CHECK: static constexpr uint16_t IntrinsicsToAttributesMap[] = {
5151
// CHECK-NEXT: 0 << 1 | 0, // llvm.aarch64.get.fpmr.set.za

0 commit comments

Comments
 (0)