Skip to content

Commit cc1ca59

Browse files
authored
[GlobalIsel] Add failure memory order to LegalityQuery (NFC) (#162284)
The `cmpxchg` instruction has two memory orders, one for success and one for failure. Prior to this patch `LegalityQuery` only exposed a single memory order, that of the success case. This meant that it was not generally possible to legalize `cmpxchg` instructions based on their memory orders. Add a `FailureOrdering` field to `LegalityQuery::MemDesc`; it is only set for `cmpxchg` instructions, otherwise it is `NotAtomic`. I didn't rename `Ordering` to `SuccessOrdering` or similar to avoid breaking changes for out of tree targets. The new field does not increase `sizeof(MemDesc)`, it falls into previous padding bits due to alignment, so I'd expect there to be no performance impact for this change. Verified no breakage via check-llvm in build with AMDGPU, AArch64, and X86 targets enabled.
1 parent fb37929 commit cc1ca59

File tree

4 files changed

+36
-26
lines changed

4 files changed

+36
-26
lines changed

llvm/include/llvm/CodeGen/GlobalISel/LegalizerInfo.h

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -115,14 +115,17 @@ struct LegalityQuery {
115115
struct MemDesc {
116116
LLT MemoryTy;
117117
uint64_t AlignInBits;
118-
AtomicOrdering Ordering;
118+
AtomicOrdering Ordering; //< For cmpxchg this is the success ordering.
119+
AtomicOrdering FailureOrdering; //< For cmpxchg, otherwise NotAtomic.
119120

120121
MemDesc() = default;
121-
MemDesc(LLT MemoryTy, uint64_t AlignInBits, AtomicOrdering Ordering)
122-
: MemoryTy(MemoryTy), AlignInBits(AlignInBits), Ordering(Ordering) {}
122+
MemDesc(LLT MemoryTy, uint64_t AlignInBits, AtomicOrdering Ordering,
123+
AtomicOrdering FailureOrdering)
124+
: MemoryTy(MemoryTy), AlignInBits(AlignInBits), Ordering(Ordering),
125+
FailureOrdering(FailureOrdering) {}
123126
MemDesc(const MachineMemOperand &MMO)
124127
: MemDesc(MMO.getMemoryType(), MMO.getAlign().value() * 8,
125-
MMO.getSuccessOrdering()) {}
128+
MMO.getSuccessOrdering(), MMO.getFailureOrdering()) {}
126129
};
127130

128131
/// Operations which require memory can use this to place requirements on the

llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1215,7 +1215,7 @@ bool CombinerHelper::isIndexedLoadStoreLegal(GLoadStore &LdSt) const {
12151215
LLT MemTy = LdSt.getMMO().getMemoryType();
12161216
SmallVector<LegalityQuery::MemDesc, 2> MemDescrs(
12171217
{{MemTy, MemTy.getSizeInBits().getKnownMinValue(),
1218-
AtomicOrdering::NotAtomic}});
1218+
AtomicOrdering::NotAtomic, AtomicOrdering::NotAtomic}});
12191219
unsigned IndexedOpc = getIndexedOpc(LdSt.getOpcode());
12201220
SmallVector<LLT> OpTys;
12211221
if (IndexedOpc == TargetOpcode::G_INDEXED_STORE)

llvm/lib/CodeGen/GlobalISel/LoadStoreOpt.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -958,7 +958,8 @@ void LoadStoreOpt::initializeStoreMergeTargetInfo(unsigned AddrSpace) {
958958
for (unsigned Size = 2; Size <= MaxStoreSizeToForm; Size *= 2) {
959959
LLT Ty = LLT::scalar(Size);
960960
SmallVector<LegalityQuery::MemDesc, 2> MemDescrs(
961-
{{Ty, Ty.getSizeInBits(), AtomicOrdering::NotAtomic}});
961+
{{Ty, Ty.getSizeInBits(), AtomicOrdering::NotAtomic,
962+
AtomicOrdering::NotAtomic}});
962963
SmallVector<LLT> StoreTys({Ty, PtrTy});
963964
LegalityQuery Q(TargetOpcode::G_STORE, StoreTys, MemDescrs);
964965
LegalizeActionStep ActionStep = LI.getAction(Q);

llvm/unittests/CodeGen/GlobalISel/LegalizerInfoTest.cpp

Lines changed: 26 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -480,18 +480,21 @@ TEST(LegalizerInfoTest, MMOAlignment) {
480480

481481
LegacyInfo.computeTables();
482482

483-
EXPECT_ACTION(Legal, 0, LLT(),
484-
LegalityQuery(G_LOAD, {s32, p0},
485-
LegalityQuery::MemDesc{
486-
s32, 32, AtomicOrdering::NotAtomic}));
487-
EXPECT_ACTION(Unsupported, 0, LLT(),
488-
LegalityQuery(G_LOAD, {s32, p0},
489-
LegalityQuery::MemDesc{
490-
s32, 16, AtomicOrdering::NotAtomic }));
491-
EXPECT_ACTION(Unsupported, 0, LLT(),
492-
LegalityQuery(G_LOAD, {s32, p0},
493-
LegalityQuery::MemDesc{
494-
s32, 8, AtomicOrdering::NotAtomic}));
483+
EXPECT_ACTION(
484+
Legal, 0, LLT(),
485+
LegalityQuery(G_LOAD, {s32, p0},
486+
LegalityQuery::MemDesc{s32, 32, AtomicOrdering::NotAtomic,
487+
AtomicOrdering::NotAtomic}));
488+
EXPECT_ACTION(
489+
Unsupported, 0, LLT(),
490+
LegalityQuery(G_LOAD, {s32, p0},
491+
LegalityQuery::MemDesc{s32, 16, AtomicOrdering::NotAtomic,
492+
AtomicOrdering::NotAtomic}));
493+
EXPECT_ACTION(
494+
Unsupported, 0, LLT(),
495+
LegalityQuery(G_LOAD, {s32, p0},
496+
LegalityQuery::MemDesc{s32, 8, AtomicOrdering::NotAtomic,
497+
AtomicOrdering::NotAtomic}));
495498
}
496499

497500
// Test that the maximum supported alignment value isn't truncated
@@ -506,14 +509,17 @@ TEST(LegalizerInfoTest, MMOAlignment) {
506509

507510
LegacyInfo.computeTables();
508511

509-
EXPECT_ACTION(Legal, 0, LLT(),
510-
LegalityQuery(G_LOAD, {s32, p0},
511-
LegalityQuery::MemDesc{s32,
512-
MaxAlignInBits, AtomicOrdering::NotAtomic}));
513-
EXPECT_ACTION(Unsupported, 0, LLT(),
514-
LegalityQuery(G_LOAD, {s32, p0},
515-
LegalityQuery::MemDesc{
516-
s32, 8, AtomicOrdering::NotAtomic }));
512+
EXPECT_ACTION(
513+
Legal, 0, LLT(),
514+
LegalityQuery(G_LOAD, {s32, p0},
515+
LegalityQuery::MemDesc{s32, MaxAlignInBits,
516+
AtomicOrdering::NotAtomic,
517+
AtomicOrdering::NotAtomic}));
518+
EXPECT_ACTION(
519+
Unsupported, 0, LLT(),
520+
LegalityQuery(G_LOAD, {s32, p0},
521+
LegalityQuery::MemDesc{s32, 8, AtomicOrdering::NotAtomic,
522+
AtomicOrdering::NotAtomic}));
517523
}
518524
}
519525

0 commit comments

Comments
 (0)