[IR] Check identical alignment for atomic instructions #155349

ellishg · 2025-08-26T03:34:59Z

I noticed that hasSameSpecialState() checks alignment for load/store instructions, but not for cmpxchg or atomicrmw, which I assume is a bug. It looks like alignment for these instructions were added in 74c7237.

llvmbot · 2025-08-26T03:35:47Z

@llvm/pr-subscribers-llvm-transforms
@llvm/pr-subscribers-llvm-analysis

@llvm/pr-subscribers-llvm-ir

Author: Ellis Hoag (ellishg)

Changes

I noticed that hasSameSpecialState() checks alignment for load/store instructions, but not for cmpxchg or atomicrmw, which I assume is a bug. It looks like alignment for these instructions were added in 74c7237.

Full diff: https://github.com/llvm/llvm-project/pull/155349.diff

2 Files Affected:

(modified) llvm/lib/IR/Instruction.cpp (+5-1)
(modified) llvm/unittests/Analysis/IRSimilarityIdentifierTest.cpp (+159)

diff --git a/llvm/lib/IR/Instruction.cpp b/llvm/lib/IR/Instruction.cpp
index 5e87b5ff941ad..6010e2a99cf01 100644
--- a/llvm/lib/IR/Instruction.cpp
+++ b/llvm/lib/IR/Instruction.cpp
@@ -864,7 +864,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");
 
@@ -917,6 +917,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() &&
@@ -927,6 +929,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))
diff --git a/llvm/unittests/Analysis/IRSimilarityIdentifierTest.cpp b/llvm/unittests/Analysis/IRSimilarityIdentifierTest.cpp
index 03009d53d63f4..067fec85e182b 100644
--- a/llvm/unittests/Analysis/IRSimilarityIdentifierTest.cpp
+++ b/llvm/unittests/Analysis/IRSimilarityIdentifierTest.cpp
@@ -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;
@@ -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(i32* %a, i64* %b) {
+                          bb0:
+                             %1 = atomicrmw add i32* %a, i32 1 acquire
+                             %2 = atomicrmw add i64* %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(i32* %a, i32* %b) {
+                          bb0:
+                             %1 = atomicrmw add i32* %a, i32 1 acquire, align 4
+                             %2 = atomicrmw add i32* %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(i32* %a, i32* %b) {
+                          bb0:
+                             %1 = atomicrmw volatile add i32* %a, i32 1 acquire
+                             %2 = atomicrmw add i32* %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(i32* %a, i64* %b) {
+                          bb0:
+                             %1 = cmpxchg i32* %a, i32 0, i32 1 monotonic monotonic
+                             %2 = cmpxchg i64* %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(i32* %a, i32* %b) {
+                          bb0:
+                             %1 = cmpxchg i32* %a, i32 0, i32 1 monotonic monotonic, align 4
+                             %2 = cmpxchg i32* %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(i32* %a, i32* %b) {
+                          bb0:
+                             %1 = cmpxchg volatile i32* %a, i32 0, i32 1 monotonic monotonic
+                             %2 = cmpxchg i32* %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"(

nikic · 2025-08-26T10:33:20Z

llvm/unittests/Analysis/IRSimilarityIdentifierTest.cpp

+// different unsigned integers.
+TEST(IRInstructionMapper, AtomicRMWDifferentType) {
+  StringRef ModuleString = R"(
+                          define i32 @f(i32* %a, i64* %b) {


Suggested change

define i32 @f(i32* %a, i64* %b) {

define i32 @f(ptr %a, ptr %b) {

etc.

ellishg · 2025-09-02T17:48:08Z

Friendly ping

ellishg · 2025-09-17T18:40:52Z

Does anyone have a comment on this possible bug fix?

nikic

LGTM, though I'd prefer testing this via lit.

nikic · 2025-09-17T19:09:47Z

llvm/unittests/Analysis/IRSimilarityIdentifierTest.cpp

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

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

Yeah, no point in having both.

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.

- Commit f9c2565 - PR #155349 LoopVectorize ones.

- Commit f9c2565 - PR llvm#155349 LoopVectorize ones.

Check identical alignment for atomic instrs

e687f1f

ellishg requested review from gchatelet, jrbyrnes and AndrewLitteken August 26, 2025 03:34

ellishg marked this pull request as ready for review August 26, 2025 03:35

llvmbot added llvm:ir llvm:analysis Includes value tracking, cost tables and constant folding labels Aug 26, 2025

nikic changed the title ~~Check identical alignment for atomic instructions~~ [IR] Check identical alignment for atomic instructions Aug 26, 2025

nikic reviewed Aug 26, 2025

View reviewed changes

use ptr

77440a9

nikic approved these changes Sep 17, 2025

View reviewed changes

ellishg added 2 commits September 18, 2025 12:01

Merge branch 'main' into fix-align-atomics

d34d65c

add iroutliner test

e44fabf

llvmbot added the llvm:transforms label Sep 18, 2025

ellishg added 2 commits September 19, 2025 10:48

Remove new unittests

e17d7ec

Merge branch 'main' into fix-align-atomics

f6dcf10

ellishg merged commit f6a14a0 into llvm:main Sep 22, 2025
9 checks passed

mtrofin added a commit that referenced this pull request Sep 29, 2025

Exclude from profcheck some tests

64a9743

- Commit f9c2565 - PR #155349 LoopVectorize ones.

mtrofin mentioned this pull request Sep 29, 2025

Exclude from profcheck some tests #161207

Merged

mtrofin added a commit that referenced this pull request Sep 29, 2025

Exclude from profcheck some tests (#161207)

1ad31d9

- Commit f9c2565 - PR #155349 LoopVectorize ones.

mahesh-attarde pushed a commit to mahesh-attarde/llvm-project that referenced this pull request Oct 3, 2025

Exclude from profcheck some tests (llvm#161207)

e77e331

- Commit f9c2565 - PR llvm#155349 LoopVectorize ones.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[IR] Check identical alignment for atomic instructions #155349

[IR] Check identical alignment for atomic instructions #155349

Uh oh!

ellishg commented Aug 26, 2025

Uh oh!

llvmbot commented Aug 26, 2025 •

edited

Loading

Uh oh!

nikic Aug 26, 2025

Uh oh!

ellishg commented Sep 2, 2025

Uh oh!

ellishg commented Sep 17, 2025

Uh oh!

nikic left a comment

Uh oh!

nikic Sep 17, 2025

Uh oh!

ellishg Sep 18, 2025

Uh oh!

nikic Sep 19, 2025

Uh oh!

ellishg Sep 19, 2025

Uh oh!

Uh oh!

Uh oh!

	define i32 @f(i32* %a, i64* %b) {
	define i32 @f(ptr %a, ptr %b) {

[IR] Check identical alignment for atomic instructions #155349

[IR] Check identical alignment for atomic instructions #155349

Uh oh!

Conversation

ellishg commented Aug 26, 2025

Uh oh!

llvmbot commented Aug 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nikic Aug 26, 2025

Choose a reason for hiding this comment

Uh oh!

ellishg commented Sep 2, 2025

Uh oh!

ellishg commented Sep 17, 2025

Uh oh!

nikic left a comment

Choose a reason for hiding this comment

Uh oh!

nikic Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!

ellishg Sep 18, 2025

Choose a reason for hiding this comment

Uh oh!

nikic Sep 19, 2025

Choose a reason for hiding this comment

Uh oh!

ellishg Sep 19, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

llvmbot commented Aug 26, 2025 •

edited

Loading