Skip to content

[IR] Check identical alignment for atomic instructions #155349

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Sep 22, 2025
Merged

[IR] Check identical alignment for atomic instructions #155349

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e687f1f
Check identical alignment for atomic instrs
ellishg Aug 26, 2025
77440a9
use ptr
ellishg Aug 26, 2025
d34d65c
Merge branch 'main' into fix-align-atomics
ellishg Sep 18, 2025
e44fabf
add iroutliner test
ellishg Sep 18, 2025
e17d7ec
Remove new unittests
ellishg Sep 19, 2025
f6dcf10
Merge branch 'main' into fix-align-atomics
ellishg Sep 19, 2025
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
6 changes: 5 additions & 1 deletion llvm/lib/IR/Instruction.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -865,7 +865,7 @@ const char *Instruction::getOpcodeName(unsigned OpCode) {
bool Instruction::hasSameSpecialState(const Instruction *I2,
bool IgnoreAlignment,
bool IntersectAttrs) const {
auto I1 = this;
const auto *I1 = this;
assert(I1->getOpcode() == I2->getOpcode() &&
"Can not compare special state of different instructions");

Expand Down Expand Up @@ -918,6 +918,8 @@ bool Instruction::hasSameSpecialState(const Instruction *I2,
FI->getSyncScopeID() == cast<FenceInst>(I2)->getSyncScopeID();
if (const AtomicCmpXchgInst *CXI = dyn_cast<AtomicCmpXchgInst>(I1))
return CXI->isVolatile() == cast<AtomicCmpXchgInst>(I2)->isVolatile() &&
(CXI->getAlign() == cast<AtomicCmpXchgInst>(I2)->getAlign() ||
IgnoreAlignment) &&
CXI->isWeak() == cast<AtomicCmpXchgInst>(I2)->isWeak() &&
CXI->getSuccessOrdering() ==
cast<AtomicCmpXchgInst>(I2)->getSuccessOrdering() &&
Expand All @@ -928,6 +930,8 @@ bool Instruction::hasSameSpecialState(const Instruction *I2,
if (const AtomicRMWInst *RMWI = dyn_cast<AtomicRMWInst>(I1))
return RMWI->getOperation() == cast<AtomicRMWInst>(I2)->getOperation() &&
RMWI->isVolatile() == cast<AtomicRMWInst>(I2)->isVolatile() &&
(RMWI->getAlign() == cast<AtomicRMWInst>(I2)->getAlign() ||
IgnoreAlignment) &&
RMWI->getOrdering() == cast<AtomicRMWInst>(I2)->getOrdering() &&
RMWI->getSyncScopeID() == cast<AtomicRMWInst>(I2)->getSyncScopeID();
if (const ShuffleVectorInst *SVI = dyn_cast<ShuffleVectorInst>(I1))
Expand Down
163 changes: 163 additions & 0 deletions llvm/test/Transforms/IROutliner/outlining-special-state.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,163 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --include-generated-funcs
; RUN: opt -S -passes=verify,iroutliner -ir-outlining-no-cost < %s | FileCheck %s

declare void @foo();

define void @atomicrmw_base(ptr %p) {
entry:
%1 = atomicrmw add ptr %p, i32 1 acquire, align 8
call void @foo()
ret void
}

define void @atomicrmw_copy(ptr %p) {
entry:
%1 = atomicrmw add ptr %p, i32 1 acquire, align 8
call void @foo()
ret void
}

define void @atomicrmw_wrong_type(ptr %p) {
entry:
%1 = atomicrmw add ptr %p, i64 1 acquire, align 8
call void @foo()
ret void
}

define void @atomicrmw_wrong_align(ptr %p) {
entry:
%1 = atomicrmw add ptr %p, i32 1 acquire, align 4
call void @foo()
ret void
}

define void @atomicrmw_wrong_volatile(ptr %p) {
entry:
%1 = atomicrmw volatile add ptr %p, i32 1 acquire, align 8
call void @foo()
ret void
}

define void @cmpxchg_base(ptr %p) {
entry:
%1 = cmpxchg ptr %p, i32 0, i32 1 monotonic monotonic, align 8
call void @foo()
ret void
}

define void @cmpxchg_copy(ptr %p) {
entry:
%1 = cmpxchg ptr %p, i32 0, i32 1 monotonic monotonic, align 8
call void @foo()
ret void
}

define void @cmpxchg_wrong_type(ptr %p) {
entry:
%1 = cmpxchg ptr %p, i64 0, i64 1 monotonic monotonic, align 8
call void @foo()
ret void
}

define void @cmpxchg_wrong_align(ptr %p) {
entry:
%1 = cmpxchg ptr %p, i32 0, i32 1 monotonic monotonic, align 4
call void @foo()
ret void
}

define void @cmpxchg_wrong_volatile(ptr %p) {
entry:
%1 = cmpxchg volatile ptr %p, i32 0, i32 1 monotonic monotonic, align 8
call void @foo()
ret void
}


; CHECK-LABEL: @atomicrmw_base(
; CHECK-NEXT: entry:
; CHECK-NEXT: call void @outlined_ir_func_1(ptr [[P:%.*]])
; CHECK-NEXT: ret void
;
;
; CHECK-LABEL: @atomicrmw_copy(
; CHECK-NEXT: entry:
; CHECK-NEXT: call void @outlined_ir_func_1(ptr [[P:%.*]])
; CHECK-NEXT: ret void
;
;
; CHECK-LABEL: @atomicrmw_wrong_type(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[TMP0:%.*]] = atomicrmw add ptr [[P:%.*]], i64 1 acquire, align 8
; CHECK-NEXT: call void @foo()
; CHECK-NEXT: ret void
;
;
; CHECK-LABEL: @atomicrmw_wrong_align(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[TMP0:%.*]] = atomicrmw add ptr [[P:%.*]], i32 1 acquire, align 4
; CHECK-NEXT: call void @foo()
; CHECK-NEXT: ret void
;
;
; CHECK-LABEL: @atomicrmw_wrong_volatile(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[TMP0:%.*]] = atomicrmw volatile add ptr [[P:%.*]], i32 1 acquire, align 8
; CHECK-NEXT: call void @foo()
; CHECK-NEXT: ret void
;
;
; CHECK-LABEL: @cmpxchg_base(
; CHECK-NEXT: entry:
; CHECK-NEXT: call void @outlined_ir_func_0(ptr [[P:%.*]])
; CHECK-NEXT: ret void
;
;
; CHECK-LABEL: @cmpxchg_copy(
; CHECK-NEXT: entry:
; CHECK-NEXT: call void @outlined_ir_func_0(ptr [[P:%.*]])
; CHECK-NEXT: ret void
;
;
; CHECK-LABEL: @cmpxchg_wrong_type(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[TMP0:%.*]] = cmpxchg ptr [[P:%.*]], i64 0, i64 1 monotonic monotonic, align 8
; CHECK-NEXT: call void @foo()
; CHECK-NEXT: ret void
;
;
; CHECK-LABEL: @cmpxchg_wrong_align(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[TMP0:%.*]] = cmpxchg ptr [[P:%.*]], i32 0, i32 1 monotonic monotonic, align 4
; CHECK-NEXT: call void @foo()
; CHECK-NEXT: ret void
;
;
; CHECK-LABEL: @cmpxchg_wrong_volatile(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[TMP0:%.*]] = cmpxchg volatile ptr [[P:%.*]], i32 0, i32 1 monotonic monotonic, align 8
; CHECK-NEXT: call void @foo()
; CHECK-NEXT: ret void
;
;
; CHECK-LABEL: @outlined_ir_func_0(
; CHECK-NEXT: newFuncRoot:
; CHECK-NEXT: br label [[ENTRY_TO_OUTLINE:%.*]]
; CHECK: entry_to_outline:
; CHECK-NEXT: [[TMP1:%.*]] = cmpxchg ptr [[TMP0:%.*]], i32 0, i32 1 monotonic monotonic, align 8
; CHECK-NEXT: call void @foo()
; CHECK-NEXT: br label [[ENTRY_AFTER_OUTLINE_EXITSTUB:%.*]]
; CHECK: entry_after_outline.exitStub:
; CHECK-NEXT: ret void
;
;
; CHECK-LABEL: @outlined_ir_func_1(
; CHECK-NEXT: newFuncRoot:
; CHECK-NEXT: br label [[ENTRY_TO_OUTLINE:%.*]]
; CHECK: entry_to_outline:
; CHECK-NEXT: [[TMP1:%.*]] = atomicrmw add ptr [[TMP0:%.*]], i32 1 acquire, align 8
; CHECK-NEXT: call void @foo()
; CHECK-NEXT: br label [[ENTRY_AFTER_OUTLINE_EXITSTUB:%.*]]
; CHECK: entry_after_outline.exitStub:
; CHECK-NEXT: ret void
;
3 changes: 3 additions & 0 deletions llvm/unittests/Analysis/IRSimilarityIdentifierTest.cpp
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we test this via SimplifyCFG lit tests for sink or hoist instead? IR based unit tests are a pain.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I created a test in Transforms/IROutliner. Should I remove these new unittests?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, no point in having both.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I ended up removing the tests I added. I didn't want to translate the existing tests to LIT because I thought that was the beyond the scope of this PR, but I did add a TODO comment. I'll land this Monday if there are no other concerns.

Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ void getSimilarities(
SimilarityCandidates = Identifier.findSimilarity(M);
}

// TODO: All these tests could probably become IR LIT tests like
// IROutliner/outlining-special-state.ll

// Checks that different opcodes are mapped to different values
TEST(IRInstructionMapper, OpcodeDifferentiation) {
StringRef ModuleString = R"(
Expand Down