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 3 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
159 changes: 159 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 @@ -19,11 +19,14 @@
#include "llvm/Support/Allocator.h"
#include "llvm/Support/Compiler.h"
#include "llvm/Support/SourceMgr.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"

using namespace llvm;
using namespace IRSimilarity;

using testing::SizeIs;

static std::unique_ptr<Module> makeLLVMModule(LLVMContext &Context,
StringRef ModuleStr) {
SMDiagnostic Err;
Expand Down Expand Up @@ -730,6 +733,162 @@ TEST(IRInstructionMapper, StoreDifferentAtomic) {
ASSERT_TRUE(UnsignedVec[0] != UnsignedVec[1]);
}

// Checks that atomicrmw that have the different types are mapped to
// different unsigned integers.
TEST(IRInstructionMapper, AtomicRMWDifferentType) {
StringRef ModuleString = R"(
define i32 @f(ptr %a, ptr %b) {
bb0:
%1 = atomicrmw add ptr %a, i32 1 acquire
%2 = atomicrmw add ptr %b, i64 1 acquire
ret i32 0
})";
LLVMContext Context;
std::unique_ptr<Module> M = makeLLVMModule(Context, ModuleString);

std::vector<IRInstructionData *> InstrList;
std::vector<unsigned> UnsignedVec;

SpecificBumpPtrAllocator<IRInstructionData> InstDataAllocator;
SpecificBumpPtrAllocator<IRInstructionDataList> IDLAllocator;
IRInstructionMapper Mapper(&InstDataAllocator, &IDLAllocator);
getVectors(*M, Mapper, InstrList, UnsignedVec);

ASSERT_EQ(InstrList.size(), UnsignedVec.size());
ASSERT_THAT(UnsignedVec, SizeIs(3));
EXPECT_NE(UnsignedVec[0], UnsignedVec[1]);
}

// Checks that atomicrmw that have the different aligns are mapped to different
// unsigned integers.
TEST(IRInstructionMapper, AtomicRMWDifferentAlign) {
StringRef ModuleString = R"(
define i32 @f(ptr %a, ptr %b) {
bb0:
%1 = atomicrmw add ptr %a, i32 1 acquire, align 4
%2 = atomicrmw add ptr %b, i32 1 acquire, align 8
ret i32 0
})";
LLVMContext Context;
std::unique_ptr<Module> M = makeLLVMModule(Context, ModuleString);

std::vector<IRInstructionData *> InstrList;
std::vector<unsigned> UnsignedVec;

SpecificBumpPtrAllocator<IRInstructionData> InstDataAllocator;
SpecificBumpPtrAllocator<IRInstructionDataList> IDLAllocator;
IRInstructionMapper Mapper(&InstDataAllocator, &IDLAllocator);
getVectors(*M, Mapper, InstrList, UnsignedVec);

ASSERT_EQ(InstrList.size(), UnsignedVec.size());
ASSERT_THAT(UnsignedVec, SizeIs(3));
EXPECT_NE(UnsignedVec[0], UnsignedVec[1]);
}

// Checks that atomicrmw that have the different volatile settings are mapped to
// different unsigned integers.
TEST(IRInstructionMapper, AtomicRMWDifferentVolatile) {
StringRef ModuleString = R"(
define i32 @f(ptr %a, ptr %b) {
bb0:
%1 = atomicrmw volatile add ptr %a, i32 1 acquire
%2 = atomicrmw add ptr %b, i32 1 acquire
ret i32 0
})";
LLVMContext Context;
std::unique_ptr<Module> M = makeLLVMModule(Context, ModuleString);

std::vector<IRInstructionData *> InstrList;
std::vector<unsigned> UnsignedVec;

SpecificBumpPtrAllocator<IRInstructionData> InstDataAllocator;
SpecificBumpPtrAllocator<IRInstructionDataList> IDLAllocator;
IRInstructionMapper Mapper(&InstDataAllocator, &IDLAllocator);
getVectors(*M, Mapper, InstrList, UnsignedVec);

ASSERT_EQ(InstrList.size(), UnsignedVec.size());
ASSERT_THAT(UnsignedVec, SizeIs(3));
EXPECT_NE(UnsignedVec[0], UnsignedVec[1]);
}

// Checks that cmpxchg that have the different types are mapped to
// different unsigned integers.
TEST(IRInstructionMapper, AtomicCmpXchgDifferentType) {
StringRef ModuleString = R"(
define i32 @f(ptr %a, ptr %b) {
bb0:
%1 = cmpxchg ptr %a, i32 0, i32 1 monotonic monotonic
%2 = cmpxchg ptr %b, i64 0, i64 1 monotonic monotonic
ret i32 0
})";
LLVMContext Context;
std::unique_ptr<Module> M = makeLLVMModule(Context, ModuleString);

std::vector<IRInstructionData *> InstrList;
std::vector<unsigned> UnsignedVec;

SpecificBumpPtrAllocator<IRInstructionData> InstDataAllocator;
SpecificBumpPtrAllocator<IRInstructionDataList> IDLAllocator;
IRInstructionMapper Mapper(&InstDataAllocator, &IDLAllocator);
getVectors(*M, Mapper, InstrList, UnsignedVec);

ASSERT_EQ(InstrList.size(), UnsignedVec.size());
ASSERT_THAT(UnsignedVec, SizeIs(3));
EXPECT_NE(UnsignedVec[0], UnsignedVec[1]);
}

// Checks that cmpxchg that have the different aligns are mapped to different
// unsigned integers.
TEST(IRInstructionMapper, AtomicCmpXchgDifferentAlign) {
StringRef ModuleString = R"(
define i32 @f(ptr %a, ptr %b) {
bb0:
%1 = cmpxchg ptr %a, i32 0, i32 1 monotonic monotonic, align 4
%2 = cmpxchg ptr %b, i32 0, i32 1 monotonic monotonic, align 8
ret i32 0
})";
LLVMContext Context;
std::unique_ptr<Module> M = makeLLVMModule(Context, ModuleString);

std::vector<IRInstructionData *> InstrList;
std::vector<unsigned> UnsignedVec;

SpecificBumpPtrAllocator<IRInstructionData> InstDataAllocator;
SpecificBumpPtrAllocator<IRInstructionDataList> IDLAllocator;
IRInstructionMapper Mapper(&InstDataAllocator, &IDLAllocator);
getVectors(*M, Mapper, InstrList, UnsignedVec);

ASSERT_EQ(InstrList.size(), UnsignedVec.size());
ASSERT_THAT(UnsignedVec, SizeIs(3));
EXPECT_NE(UnsignedVec[0], UnsignedVec[1]);
}

// Checks that cmpxchg that have the different volatile settings are mapped to
// different unsigned integers.
TEST(IRInstructionMapper, AtomicCmpXchgDifferentVolatile) {
StringRef ModuleString = R"(
define i32 @f(ptr %a, ptr %b) {
bb0:
%1 = cmpxchg volatile ptr %a, i32 0, i32 1 monotonic monotonic
%2 = cmpxchg ptr %b, i32 0, i32 1 monotonic monotonic
ret i32 0
})";
LLVMContext Context;
std::unique_ptr<Module> M = makeLLVMModule(Context, ModuleString);

std::vector<IRInstructionData *> InstrList;
std::vector<unsigned> UnsignedVec;

SpecificBumpPtrAllocator<IRInstructionData> InstDataAllocator;
SpecificBumpPtrAllocator<IRInstructionDataList> IDLAllocator;
IRInstructionMapper Mapper(&InstDataAllocator, &IDLAllocator);
getVectors(*M, Mapper, InstrList, UnsignedVec);

ASSERT_EQ(InstrList.size(), UnsignedVec.size());
ASSERT_THAT(UnsignedVec, SizeIs(3));
EXPECT_NE(UnsignedVec[0], UnsignedVec[1]);
}

// Checks that the branch is mapped to legal when the option is set.
TEST(IRInstructionMapper, BranchLegal) {
StringRef ModuleString = R"(
Expand Down