[llvm] [SeparateConstOffsetFromGEP] Decompose constant xor operand if possible (PR #135788)
Sumanth Gundapaneni via llvm-commits
llvm-commits at lists.llvm.org
Thu May 29 12:24:10 PDT 2025
https://github.com/sgundapa updated https://github.com/llvm/llvm-project/pull/135788
>From e45f0aadcd864d4bd8d1bf2bfb1d7c2d486103a8 Mon Sep 17 00:00:00 2001
From: Sumanth Gundapaneni <sugundap at amd.com>
Date: Tue, 15 Apr 2025 08:31:38 -0500
Subject: [PATCH 1/8] Decompose Xors that are fed to GEPs
NOTE: This patch is not to be merged, just for evaluation.
---
.../Scalar/SeparateConstOffsetFromGEP.cpp | 166 ++++++++++++++++++
1 file changed, 166 insertions(+)
diff --git a/llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp b/llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp
index e048015298461..b0f7c7d862519 100644
--- a/llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp
+++ b/llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp
@@ -160,6 +160,7 @@
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/DepthFirstIterator.h"
#include "llvm/ADT/SmallVector.h"
+#include "llvm/Analysis/AssumptionCache.h"
#include "llvm/Analysis/LoopInfo.h"
#include "llvm/Analysis/MemoryBuiltins.h"
#include "llvm/Analysis/TargetLibraryInfo.h"
@@ -198,6 +199,8 @@
using namespace llvm;
using namespace llvm::PatternMatch;
+#define DEBUG_TYPE "separate-offset-gep"
+
static cl::opt<bool> DisableSeparateConstOffsetFromGEP(
"disable-separate-const-offset-from-gep", cl::init(false),
cl::desc("Do not separate the constant offset from a GEP instruction"),
@@ -484,6 +487,9 @@ class SeparateConstOffsetFromGEP {
DenseMap<ExprKey, SmallVector<Instruction *, 2>> DominatingAdds;
DenseMap<ExprKey, SmallVector<Instruction *, 2>> DominatingSubs;
+
+ bool decomposeXor(Function &F);
+ Value *tryFoldXorToOrDisjoint(Instruction &I);
};
} // end anonymous namespace
@@ -1162,6 +1168,162 @@ bool SeparateConstOffsetFromGEP::splitGEP(GetElementPtrInst *GEP) {
return true;
}
+bool SeparateConstOffsetFromGEP::decomposeXor(Function &F) {
+ bool FunctionChanged = false;
+ SmallVector<std::pair<Instruction *, Value *>, 16> ReplacementsToMake;
+
+ for (BasicBlock &BB : F) {
+ for (Instruction &I : BB) {
+ if (I.getOpcode() == Instruction::Xor) {
+ if (Value *Replacement = tryFoldXorToOrDisjoint(I)) {
+ ReplacementsToMake.push_back({&I, Replacement});
+ FunctionChanged = true;
+ }
+ }
+ }
+ }
+
+ if (!ReplacementsToMake.empty()) {
+ LLVM_DEBUG(dbgs() << "Applying " << ReplacementsToMake.size()
+ << " XOR->OR Disjoint replacements in " << F.getName()
+ << "\n");
+ for (auto &Pair : ReplacementsToMake) {
+ Pair.first->replaceAllUsesWith(Pair.second);
+ }
+ for (auto &Pair : ReplacementsToMake) {
+ Pair.first->eraseFromParent();
+ }
+ }
+
+ return FunctionChanged;
+}
+
+static llvm::Instruction *findClosestSequentialXor(Value *A, Instruction &I) {
+ llvm::Instruction *ClosestUser = nullptr;
+ for (llvm::User *User : A->users()) {
+ if (auto *UserInst = llvm::dyn_cast<llvm::Instruction>(User)) {
+ if (UserInst->getOpcode() != Instruction::Xor || UserInst == &I)
+ continue;
+ if (!ClosestUser) {
+ ClosestUser = UserInst;
+ } else {
+ // Compare instruction positions.
+ if (UserInst->comesBefore(ClosestUser)) {
+ ClosestUser = UserInst;
+ }
+ }
+ }
+ }
+ return ClosestUser;
+}
+
+/// Try to transform I = xor(A, C1) into or disjoint(Y, C2)
+/// where Y = xor(A, C0) is another existing instruction dominating I,
+/// C2 = C0 ^ C1, and A is known to be disjoint with C2.
+///
+/// @param I The XOR instruction being visited.
+/// @return The replacement Value* if successful, nullptr otherwise.
+Value *SeparateConstOffsetFromGEP::tryFoldXorToOrDisjoint(Instruction &I) {
+ assert(I.getOpcode() == Instruction::Xor && "Instruction must be XOR");
+
+ // Check if I has at least one GEP user.
+ bool HasGepUser = false;
+ for (User *U : I.users()) {
+ if (isa<GetElementPtrInst>(U)) {
+ HasGepUser = true;
+ break;
+ }
+ }
+ // If no user is a GEP instruction, abort the transformation.
+ if (!HasGepUser) {
+ LLVM_DEBUG(
+ dbgs() << "SeparateConstOffsetFromGEP: Skipping XOR->OR DISJOINT for "
+ << I << " because it has no GEP users.\n");
+ return nullptr;
+ }
+
+ Value *Op0 = I.getOperand(0);
+ Value *Op1 = I.getOperand(1);
+ ConstantInt *C1 = dyn_cast<ConstantInt>(Op1);
+ Value *A = Op0;
+
+ // Bail out of there is not constant operand.
+ if (!C1) {
+ C1 = dyn_cast<ConstantInt>(Op0);
+ if (!C1)
+ return nullptr;
+ A = Op1;
+ }
+
+ if (isa<UndefValue>(A))
+ return nullptr;
+
+ APInt C1_APInt = C1->getValue();
+ unsigned BitWidth = C1_APInt.getBitWidth();
+ Type *Ty = I.getType();
+
+ // --- Step 2: Find Dominating Y = xor A, C0 ---
+ Instruction *FoundUserInst = nullptr; // Instruction Y
+ APInt C0_APInt;
+
+ auto UserInst = findClosestSequentialXor(A, I);
+
+ BinaryOperator *UserBO = cast<BinaryOperator>(UserInst);
+ Value *UserOp0 = UserBO->getOperand(0);
+ Value *UserOp1 = UserBO->getOperand(1);
+ ConstantInt *UserC = nullptr;
+ if (UserOp0 == A)
+ UserC = dyn_cast<ConstantInt>(UserOp1);
+ else if (UserOp1 == A)
+ UserC = dyn_cast<ConstantInt>(UserOp0);
+ if (UserC) {
+ if (DT->dominates(UserInst, &I)) {
+ FoundUserInst = UserInst;
+ C0_APInt = UserC->getValue();
+ }
+ }
+ if (!FoundUserInst)
+ return nullptr;
+
+ // Calculate C2.
+ APInt C2_APInt = C0_APInt ^ C1_APInt;
+
+ // Check Disjointness A & C2 == 0.
+ KnownBits KnownA(BitWidth);
+ AssumptionCache *AC = nullptr;
+ computeKnownBits(A, KnownA, *DL, 0, AC, &I, DT);
+
+ if ((KnownA.Zero & C2_APInt) != C2_APInt)
+ return nullptr;
+
+ IRBuilder<> Builder(&I);
+ Builder.SetInsertPoint(&I); // Access Builder directly
+ Constant *C2_Const = ConstantInt::get(Ty, C2_APInt);
+ Twine Name = I.getName(); // Create Twine explicitly
+ Value *NewOr = BinaryOperator::CreateDisjointOr(FoundUserInst, C2_Const, Name,
+ I.getIterator());
+ // Transformation Conditions Met.
+ LLVM_DEBUG(dbgs() << "SeparateConstOffsetFromGEP: Replacing " << I
+ << " (used by GEP) with " << *NewOr << " based on "
+ << *FoundUserInst << "\n");
+
+#if 0
+ // Preserve metadata
+ if (Instruction *NewOrInst = dyn_cast<Instruction>(NewOr)) {
+ NewOrInst->copyMetadata(I);
+ } else {
+ assert(false && "CreateNUWOr did not return an Instruction");
+ if (NewOr)
+ NewOr->deleteValue();
+ return nullptr;
+ }
+#endif
+
+ // Return the replacement value. runOnFunction will handle replacement &
+ // deletion.
+ return NewOr;
+}
+
bool SeparateConstOffsetFromGEPLegacyPass::runOnFunction(Function &F) {
if (skipFunction(F))
return false;
@@ -1181,6 +1343,10 @@ bool SeparateConstOffsetFromGEP::run(Function &F) {
DL = &F.getDataLayout();
bool Changed = false;
+
+ // Decompose xor in to "or disjoint" if possible.
+ Changed |= decomposeXor(F);
+
for (BasicBlock &B : F) {
if (!DT->isReachableFromEntry(&B))
continue;
>From def3b3e722b695aeb79eda829dbb45eeca7d73c2 Mon Sep 17 00:00:00 2001
From: Sumanth Gundapaneni <sugundap at amd.com>
Date: Mon, 21 Apr 2025 16:05:53 -0500
Subject: [PATCH 2/8] Push correctness fixes
---
.../Scalar/SeparateConstOffsetFromGEP.cpp | 100 ++++++-----
.../AMDGPU/xor-to-or-disjoint.ll | 163 ++++++++++++++++++
2 files changed, 221 insertions(+), 42 deletions(-)
create mode 100644 llvm/test/Transforms/SeparateConstOffsetFromGEP/AMDGPU/xor-to-or-disjoint.ll
diff --git a/llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp b/llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp
index b0f7c7d862519..19f166e5f6595 100644
--- a/llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp
+++ b/llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp
@@ -160,7 +160,6 @@
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/DepthFirstIterator.h"
#include "llvm/ADT/SmallVector.h"
-#include "llvm/Analysis/AssumptionCache.h"
#include "llvm/Analysis/LoopInfo.h"
#include "llvm/Analysis/MemoryBuiltins.h"
#include "llvm/Analysis/TargetLibraryInfo.h"
@@ -1187,12 +1186,11 @@ bool SeparateConstOffsetFromGEP::decomposeXor(Function &F) {
LLVM_DEBUG(dbgs() << "Applying " << ReplacementsToMake.size()
<< " XOR->OR Disjoint replacements in " << F.getName()
<< "\n");
- for (auto &Pair : ReplacementsToMake) {
+ for (auto &Pair : ReplacementsToMake)
Pair.first->replaceAllUsesWith(Pair.second);
- }
- for (auto &Pair : ReplacementsToMake) {
+
+ for (auto &Pair : ReplacementsToMake)
Pair.first->eraseFromParent();
- }
}
return FunctionChanged;
@@ -1204,9 +1202,9 @@ static llvm::Instruction *findClosestSequentialXor(Value *A, Instruction &I) {
if (auto *UserInst = llvm::dyn_cast<llvm::Instruction>(User)) {
if (UserInst->getOpcode() != Instruction::Xor || UserInst == &I)
continue;
- if (!ClosestUser) {
+ if (!ClosestUser)
ClosestUser = UserInst;
- } else {
+ else {
// Compare instruction positions.
if (UserInst->comesBefore(ClosestUser)) {
ClosestUser = UserInst;
@@ -1217,9 +1215,14 @@ static llvm::Instruction *findClosestSequentialXor(Value *A, Instruction &I) {
return ClosestUser;
}
-/// Try to transform I = xor(A, C1) into or disjoint(Y, C2)
+/// Try to transform I = xor(A, C1) into or_disjoint(Y, C2)
/// where Y = xor(A, C0) is another existing instruction dominating I,
-/// C2 = C0 ^ C1, and A is known to be disjoint with C2.
+/// C2 = C1 - C0, and A is known to be disjoint with C2.
+///
+/// This transformation is beneficial particularly for GEPs because:
+/// 1. OR operations often map better to addressing modes than XOR
+/// 2. Disjoint OR operations preserve the semantics of the original XOR
+/// 3. This can enable further optimizations in the GEP offset folding pipeline
///
/// @param I The XOR instruction being visited.
/// @return The replacement Value* if successful, nullptr otherwise.
@@ -1237,7 +1240,7 @@ Value *SeparateConstOffsetFromGEP::tryFoldXorToOrDisjoint(Instruction &I) {
// If no user is a GEP instruction, abort the transformation.
if (!HasGepUser) {
LLVM_DEBUG(
- dbgs() << "SeparateConstOffsetFromGEP: Skipping XOR->OR DISJOINT for "
+ dbgs() << "SeparateConstOffsetFromGEP: Skipping XOR->OR DISJOINT for"
<< I << " because it has no GEP users.\n");
return nullptr;
}
@@ -1262,11 +1265,18 @@ Value *SeparateConstOffsetFromGEP::tryFoldXorToOrDisjoint(Instruction &I) {
unsigned BitWidth = C1_APInt.getBitWidth();
Type *Ty = I.getType();
- // --- Step 2: Find Dominating Y = xor A, C0 ---
- Instruction *FoundUserInst = nullptr; // Instruction Y
+ // Find Dominating Y = xor A, C0
+ Instruction *FoundUserInst = nullptr;
APInt C0_APInt;
- auto UserInst = findClosestSequentialXor(A, I);
+ // Find the closest XOR instruction using the same value.
+ Instruction *UserInst = findClosestSequentialXor(A, I);
+ if (!UserInst) {
+ LLVM_DEBUG(
+ dbgs() << "SeparateConstOffsetFromGEP: No dominating XOR found for" << I
+ << "\n");
+ return nullptr;
+ }
BinaryOperator *UserBO = cast<BinaryOperator>(UserInst);
Value *UserOp0 = UserBO->getOperand(0);
@@ -1276,51 +1286,57 @@ Value *SeparateConstOffsetFromGEP::tryFoldXorToOrDisjoint(Instruction &I) {
UserC = dyn_cast<ConstantInt>(UserOp1);
else if (UserOp1 == A)
UserC = dyn_cast<ConstantInt>(UserOp0);
- if (UserC) {
- if (DT->dominates(UserInst, &I)) {
- FoundUserInst = UserInst;
- C0_APInt = UserC->getValue();
- }
+ else {
+ LLVM_DEBUG(dbgs() << "SeparateConstOffsetFromGEP: Found XOR" << *UserInst
+ << " doesn't use value " << *A << "\n");
+ return nullptr;
}
- if (!FoundUserInst)
+
+ if (!UserC) {
+ LLVM_DEBUG(
+ dbgs()
+ << "SeparateConstOffsetFromGEP: Found XOR doesn't have constant operand"
+ << *UserInst << "\n");
return nullptr;
+ }
- // Calculate C2.
- APInt C2_APInt = C0_APInt ^ C1_APInt;
+ if (!DT->dominates(UserInst, &I)) {
+ LLVM_DEBUG(dbgs() << "SeparateConstOffsetFromGEP: Found XOR" << *UserInst
+ << " doesn't dominate " << I << "\n");
+ return nullptr;
+ }
+
+ FoundUserInst = UserInst;
+ C0_APInt = UserC->getValue();
+
+ // Calculate C2 = C1 - C0.
+ APInt C2_APInt = C1_APInt - C0_APInt;
// Check Disjointness A & C2 == 0.
KnownBits KnownA(BitWidth);
- AssumptionCache *AC = nullptr;
- computeKnownBits(A, KnownA, *DL, 0, AC, &I, DT);
+ computeKnownBits(A, KnownA, *DL, 0, nullptr, &I, DT);
- if ((KnownA.Zero & C2_APInt) != C2_APInt)
+ if ((KnownA.One & C2_APInt) != 0) {
+ LLVM_DEBUG(
+ dbgs() << "SeparateConstOffsetFromGEP: Disjointness check failed for"
+ << I << "\n");
return nullptr;
+ }
IRBuilder<> Builder(&I);
- Builder.SetInsertPoint(&I); // Access Builder directly
+ Builder.SetInsertPoint(&I);
Constant *C2_Const = ConstantInt::get(Ty, C2_APInt);
- Twine Name = I.getName(); // Create Twine explicitly
+ Twine Name = I.getName();
Value *NewOr = BinaryOperator::CreateDisjointOr(FoundUserInst, C2_Const, Name,
I.getIterator());
- // Transformation Conditions Met.
- LLVM_DEBUG(dbgs() << "SeparateConstOffsetFromGEP: Replacing " << I
- << " (used by GEP) with " << *NewOr << " based on "
- << *FoundUserInst << "\n");
-
-#if 0
// Preserve metadata
- if (Instruction *NewOrInst = dyn_cast<Instruction>(NewOr)) {
+ if (Instruction *NewOrInst = dyn_cast<Instruction>(NewOr))
NewOrInst->copyMetadata(I);
- } else {
- assert(false && "CreateNUWOr did not return an Instruction");
- if (NewOr)
- NewOr->deleteValue();
- return nullptr;
- }
-#endif
- // Return the replacement value. runOnFunction will handle replacement &
- // deletion.
+ LLVM_DEBUG(dbgs() << "SeparateConstOffsetFromGEP: Replacing" << I
+ << " (used by GEP) with" << *NewOr << " based on"
+ << *FoundUserInst << "\n");
+
return NewOr;
}
diff --git a/llvm/test/Transforms/SeparateConstOffsetFromGEP/AMDGPU/xor-to-or-disjoint.ll b/llvm/test/Transforms/SeparateConstOffsetFromGEP/AMDGPU/xor-to-or-disjoint.ll
new file mode 100644
index 0000000000000..808df95116f12
--- /dev/null
+++ b/llvm/test/Transforms/SeparateConstOffsetFromGEP/AMDGPU/xor-to-or-disjoint.ll
@@ -0,0 +1,163 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN: opt -mtriple=amdgcn-amd-amdhsa -passes=separate-const-offset-from-gep \
+; RUN: -S < %s | FileCheck %s
+
+
+; Test with GEP user and known bits: Ensure the transformation occurs when the xor has a GEP user
+define ptr @test_with_gep_user(ptr %ptr) {
+; CHECK-LABEL: define ptr @test_with_gep_user(
+; CHECK-SAME: ptr [[PTR:%.*]]) {
+; CHECK-NEXT: [[ENTRY:.*:]]
+; CHECK-NEXT: [[BASE:%.*]] = add i64 0, 0
+; CHECK-NEXT: [[XOR1:%.*]] = xor i64 [[BASE]], 8
+; CHECK-NEXT: [[XOR21:%.*]] = or disjoint i64 [[XOR1]], 16
+; CHECK-NEXT: [[GEP:%.*]] = getelementptr i8, ptr [[PTR]], i64 [[XOR21]]
+; CHECK-NEXT: ret ptr [[GEP]]
+;
+entry:
+ %base = add i64 0,0
+ %xor1 = xor i64 %base, 8
+ %xor2 = xor i64 %base, 24 ; Should be replaced with OR of %xor1 and 16
+ %gep = getelementptr i8, ptr %ptr, i64 %xor2
+ ret ptr %gep
+}
+
+
+; Test with non-GEP user: Ensure the transformation does not occur
+define i32 @test_with_non_gep_user(ptr %ptr) {
+; CHECK-LABEL: define i32 @test_with_non_gep_user(
+; CHECK-SAME: ptr [[PTR:%.*]]) {
+; CHECK-NEXT: [[ENTRY:.*:]]
+; CHECK-NEXT: [[BASE:%.*]] = add i32 0, 0
+; CHECK-NEXT: [[XOR1:%.*]] = xor i32 [[BASE]], 8
+; CHECK-NEXT: [[XOR2:%.*]] = xor i32 [[BASE]], 24
+; CHECK-NEXT: [[ADD:%.*]] = add i32 [[XOR2]], 5
+; CHECK-NEXT: ret i32 [[ADD]]
+;
+entry:
+ %base = add i32 0,0
+ %xor1 = xor i32 %base, 8
+ %xor2 = xor i32 %base, 24
+ %add = add i32 %xor2, 5
+ ret i32 %add
+}
+
+; Test with non-constant operand: Ensure the transformation does not occur
+define ptr @test_with_non_constant_operand(i64 %val, i64 %val2, ptr %ptr) {
+; CHECK-LABEL: define ptr @test_with_non_constant_operand(
+; CHECK-SAME: i64 [[VAL:%.*]], i64 [[VAL2:%.*]], ptr [[PTR:%.*]]) {
+; CHECK-NEXT: [[ENTRY:.*:]]
+; CHECK-NEXT: [[XOR1:%.*]] = xor i64 [[VAL]], [[VAL2]]
+; CHECK-NEXT: [[XOR2:%.*]] = xor i64 [[VAL]], 24
+; CHECK-NEXT: [[GEP:%.*]] = getelementptr i8, ptr [[PTR]], i64 [[XOR2]]
+; CHECK-NEXT: ret ptr [[GEP]]
+;
+entry:
+ %xor1 = xor i64 %val, %val2 ; Non-constant operand
+ %xor2 = xor i64 %val, 24
+ %gep = getelementptr i8, ptr %ptr, i64 %xor2
+ ret ptr %gep
+}
+
+; Test with unknown disjoint bits: Ensure the transformation does not occur
+define ptr @test_with_unknown_disjoint_bits(i64 %base, ptr %ptr) {
+; CHECK-LABEL: define ptr @test_with_unknown_disjoint_bits(
+; CHECK-SAME: i64 [[BASE:%.*]], ptr [[PTR:%.*]]) {
+; CHECK-NEXT: [[ENTRY:.*:]]
+; CHECK-NEXT: [[XOR1:%.*]] = xor i64 [[BASE]], 8
+; CHECK-NEXT: [[XOR21:%.*]] = or disjoint i64 [[XOR1]], 16
+; CHECK-NEXT: [[GEP:%.*]] = getelementptr i8, ptr [[PTR]], i64 [[XOR21]]
+; CHECK-NEXT: ret ptr [[GEP]]
+;
+entry:
+ %xor1 = xor i64 %base, 8
+ %xor2 = xor i64 %base, 24
+ %gep = getelementptr i8, ptr %ptr, i64 %xor2
+ ret ptr %gep
+}
+
+; Test with non-disjoint bits: Ensure the transformation does not occur
+define ptr @test_with_non_disjoint_bits(i64 %val, ptr %ptr) {
+; CHECK-LABEL: define ptr @test_with_non_disjoint_bits(
+; CHECK-SAME: i64 [[VAL:%.*]], ptr [[PTR:%.*]]) {
+; CHECK-NEXT: [[ENTRY:.*:]]
+; CHECK-NEXT: [[AND:%.*]] = and i64 [[VAL]], 31
+; CHECK-NEXT: [[XOR1:%.*]] = xor i64 [[AND]], 4
+; CHECK-NEXT: [[XOR21:%.*]] = or disjoint i64 [[XOR1]], 16
+; CHECK-NEXT: [[GEP:%.*]] = getelementptr i8, ptr [[PTR]], i64 [[XOR21]]
+; CHECK-NEXT: ret ptr [[GEP]]
+;
+entry:
+ %and = and i64 %val, 31 ; val can have bits 0-4 set
+ %xor1 = xor i64 %and, 4 ; Flips bit 2
+ %xor2 = xor i64 %and, 20 ; Flips bits 2 and 4, should NOT replace since bit 4 overlaps with possible val bits
+ %gep = getelementptr i8, ptr %ptr, i64 %xor2
+ ret ptr %gep
+}
+
+; Test with multiple xor operations in sequence
+define ptr @test_multiple_xors(ptr %ptr) {
+; CHECK-LABEL: define ptr @test_multiple_xors(
+; CHECK-SAME: ptr [[PTR:%.*]]) {
+; CHECK-NEXT: [[ENTRY:.*:]]
+; CHECK-NEXT: [[BASE:%.*]] = add i64 2, 0
+; CHECK-NEXT: [[XOR1:%.*]] = xor i64 [[BASE]], 8
+; CHECK-NEXT: [[XOR21:%.*]] = or disjoint i64 [[XOR1]], 16
+; CHECK-NEXT: [[XOR32:%.*]] = or disjoint i64 [[XOR1]], 24
+; CHECK-NEXT: [[XOR43:%.*]] = or disjoint i64 [[XOR1]], 64
+; CHECK-NEXT: [[GEP2:%.*]] = getelementptr i8, ptr [[PTR]], i64 [[XOR21]]
+; CHECK-NEXT: [[GEP3:%.*]] = getelementptr i8, ptr [[PTR]], i64 [[XOR32]]
+; CHECK-NEXT: [[GEP4:%.*]] = getelementptr i8, ptr [[PTR]], i64 [[XOR43]]
+; CHECK-NEXT: ret ptr [[GEP4]]
+;
+entry:
+ %base = add i64 2,0
+ %xor1 = xor i64 %base, 8
+ %xor2 = xor i64 %base, 24 ; Should be replaced with OR
+ %xor3 = xor i64 %base, 32
+ %xor4 = xor i64 %base, 72 ; Should be replaced with OR
+ %gep2 = getelementptr i8, ptr %ptr, i64 %xor2
+ %gep3 = getelementptr i8, ptr %ptr, i64 %xor3
+ %gep4 = getelementptr i8, ptr %ptr, i64 %xor4
+ ret ptr %gep4
+}
+
+
+; Test with operand order variations
+define ptr @test_operand_order(ptr %ptr) {
+; CHECK-LABEL: define ptr @test_operand_order(
+; CHECK-SAME: ptr [[PTR:%.*]]) {
+; CHECK-NEXT: [[ENTRY:.*:]]
+; CHECK-NEXT: [[BASE:%.*]] = add i64 2, 0
+; CHECK-NEXT: [[XOR1:%.*]] = xor i64 [[BASE]], 12
+; CHECK-NEXT: [[XOR21:%.*]] = or disjoint i64 [[XOR1]], 12
+; CHECK-NEXT: [[GEP:%.*]] = getelementptr i8, ptr [[PTR]], i64 [[XOR21]]
+; CHECK-NEXT: ret ptr [[GEP]]
+;
+entry:
+ %base = add i64 2,0
+ %xor1 = xor i64 %base, 12
+ %xor2 = xor i64 24, %base ; Operands reversed, should still be replaced
+ %gep = getelementptr i8, ptr %ptr, i64 %xor2
+ ret ptr %gep
+}
+
+
+; Test with multiple xor operations in sequence
+define ptr @aatest_multiple_xors(ptr %ptr) {
+; CHECK-LABEL: define ptr @aatest_multiple_xors(
+; CHECK-SAME: ptr [[PTR:%.*]]) {
+; CHECK-NEXT: [[ENTRY:.*:]]
+; CHECK-NEXT: [[BASE:%.*]] = add i64 2, 0
+; CHECK-NEXT: [[XOR1:%.*]] = xor i64 [[BASE]], 72
+; CHECK-NEXT: [[XOR21:%.*]] = or disjoint i64 [[XOR1]], -48
+; CHECK-NEXT: [[GEP:%.*]] = getelementptr i8, ptr [[PTR]], i64 [[XOR21]]
+; CHECK-NEXT: ret ptr [[GEP]]
+;
+entry:
+ %base = add i64 2,0
+ %xor1 = xor i64 %base, 72
+ %xor2 = xor i64 %base, 24 ; Should be replaced with OR
+ %gep = getelementptr i8, ptr %ptr, i64 %xor2
+ ret ptr %gep
+}
>From 3492929677473ae2bc59df6b70da6e3632494103 Mon Sep 17 00:00:00 2001
From: Sumanth Gundapaneni <sugundap at amd.com>
Date: Mon, 21 Apr 2025 16:29:00 -0500
Subject: [PATCH 3/8] Update lit test
---
.../AMDGPU/xor-to-or-disjoint.ll | 23 ++-----------------
1 file changed, 2 insertions(+), 21 deletions(-)
diff --git a/llvm/test/Transforms/SeparateConstOffsetFromGEP/AMDGPU/xor-to-or-disjoint.ll b/llvm/test/Transforms/SeparateConstOffsetFromGEP/AMDGPU/xor-to-or-disjoint.ll
index 808df95116f12..d1a04a70d4994 100644
--- a/llvm/test/Transforms/SeparateConstOffsetFromGEP/AMDGPU/xor-to-or-disjoint.ll
+++ b/llvm/test/Transforms/SeparateConstOffsetFromGEP/AMDGPU/xor-to-or-disjoint.ll
@@ -76,25 +76,6 @@ entry:
ret ptr %gep
}
-; Test with non-disjoint bits: Ensure the transformation does not occur
-define ptr @test_with_non_disjoint_bits(i64 %val, ptr %ptr) {
-; CHECK-LABEL: define ptr @test_with_non_disjoint_bits(
-; CHECK-SAME: i64 [[VAL:%.*]], ptr [[PTR:%.*]]) {
-; CHECK-NEXT: [[ENTRY:.*:]]
-; CHECK-NEXT: [[AND:%.*]] = and i64 [[VAL]], 31
-; CHECK-NEXT: [[XOR1:%.*]] = xor i64 [[AND]], 4
-; CHECK-NEXT: [[XOR21:%.*]] = or disjoint i64 [[XOR1]], 16
-; CHECK-NEXT: [[GEP:%.*]] = getelementptr i8, ptr [[PTR]], i64 [[XOR21]]
-; CHECK-NEXT: ret ptr [[GEP]]
-;
-entry:
- %and = and i64 %val, 31 ; val can have bits 0-4 set
- %xor1 = xor i64 %and, 4 ; Flips bit 2
- %xor2 = xor i64 %and, 20 ; Flips bits 2 and 4, should NOT replace since bit 4 overlaps with possible val bits
- %gep = getelementptr i8, ptr %ptr, i64 %xor2
- ret ptr %gep
-}
-
; Test with multiple xor operations in sequence
define ptr @test_multiple_xors(ptr %ptr) {
; CHECK-LABEL: define ptr @test_multiple_xors(
@@ -144,8 +125,8 @@ entry:
; Test with multiple xor operations in sequence
-define ptr @aatest_multiple_xors(ptr %ptr) {
-; CHECK-LABEL: define ptr @aatest_multiple_xors(
+define ptr @test_negative_offset(ptr %ptr) {
+; CHECK-LABEL: define ptr @test_negative_offset(
; CHECK-SAME: ptr [[PTR:%.*]]) {
; CHECK-NEXT: [[ENTRY:.*:]]
; CHECK-NEXT: [[BASE:%.*]] = add i64 2, 0
>From 7ec58957b70b2fe4b74a07f77c47174fdd031ebf Mon Sep 17 00:00:00 2001
From: Sumanth Gundapaneni <sugundap at amd.com>
Date: Tue, 13 May 2025 14:11:23 -0500
Subject: [PATCH 4/8] [SeparateConstOffsetFromGEP] Decompose constant xor
operand if possible
Try to transform XOR(A, B+C) in to XOR(A,C) + B where XOR(A,C) becomes
the base for memory operations. This transformation is true under the
following conditions
Check 1 - B and C are disjoint.
Check 2 - XOR(A,C) and B are disjoint.
This transformation is beneficial particularly for GEPs because
Disjoint OR operations often map better to addressing modes than XOR.
This can enable further optimizations in the GEP offset folding pipeline
---
.../Scalar/SeparateConstOffsetFromGEP.cpp | 322 ++++++++++--------
.../AMDGPU/xor-to-or-disjoint.ll | 240 ++++++-------
2 files changed, 297 insertions(+), 265 deletions(-)
diff --git a/llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp b/llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp
index 19f166e5f6595..eb1d85584c0f6 100644
--- a/llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp
+++ b/llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp
@@ -174,6 +174,7 @@
#include "llvm/IR/Function.h"
#include "llvm/IR/GetElementPtrTypeIterator.h"
#include "llvm/IR/IRBuilder.h"
+#include "llvm/IR/InstIterator.h"
#include "llvm/IR/InstrTypes.h"
#include "llvm/IR/Instruction.h"
#include "llvm/IR/Instructions.h"
@@ -190,6 +191,7 @@
#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/raw_ostream.h"
#include "llvm/Transforms/Scalar.h"
+#include "llvm/Transforms/Utils/BasicBlockUtils.h"
#include "llvm/Transforms/Utils/Local.h"
#include <cassert>
#include <cstdint>
@@ -491,6 +493,39 @@ class SeparateConstOffsetFromGEP {
Value *tryFoldXorToOrDisjoint(Instruction &I);
};
+/// A helper class that aims to convert xor operations into or operations when
+/// their operands are disjoint and the result is used in a GEP's index. This
+/// can then enable further GEP optimizations by effectively turning BaseVal |
+/// Const into BaseVal + Const when they are disjoint, which
+/// SeparateConstOffsetFromGEP can then process. This is a common pattern that
+/// sets up a grid of memory accesses across a wave where each thread acesses
+/// data at various offsets.
+class XorToOrDisjointTransformer {
+public:
+ XorToOrDisjointTransformer(Function &F, DominatorTree &DT,
+ const DataLayout &DL)
+ : F(F), DT(DT), DL(DL) {}
+
+ bool run();
+
+private:
+ Function &F;
+ DominatorTree &DT;
+ const DataLayout &DL;
+ /// Maps a common operand to all Xor instructions
+ using XorOpList = SmallVector<std::pair<BinaryOperator *, APInt>, 8>;
+ using XorBaseValMap = DenseMap<Value *, XorOpList>;
+ XorBaseValMap XorGroups;
+
+ /// Checks if the given value has at least one GetElementPtr user
+ bool hasGEPUser(const Value *V) const;
+
+ /// Processes a group of XOR instructions that share the same non-constant
+ /// base operand. Returns true if this group's processing modified the
+ /// function.
+ bool processXorGroup(Value *OriginalBaseVal, XorOpList &XorsInGroup);
+};
+
} // end anonymous namespace
char SeparateConstOffsetFromGEPLegacyPass::ID = 0;
@@ -1167,177 +1202,163 @@ bool SeparateConstOffsetFromGEP::splitGEP(GetElementPtrInst *GEP) {
return true;
}
-bool SeparateConstOffsetFromGEP::decomposeXor(Function &F) {
- bool FunctionChanged = false;
- SmallVector<std::pair<Instruction *, Value *>, 16> ReplacementsToMake;
-
- for (BasicBlock &BB : F) {
- for (Instruction &I : BB) {
- if (I.getOpcode() == Instruction::Xor) {
- if (Value *Replacement = tryFoldXorToOrDisjoint(I)) {
- ReplacementsToMake.push_back({&I, Replacement});
- FunctionChanged = true;
- }
- }
+// Helper function to check if an instruction has at least one GEP user
+bool XorToOrDisjointTransformer::hasGEPUser(const Value *V) const {
+ for (const User *U : V->users()) {
+ if (isa<GetElementPtrInst>(U)) {
+ return true;
}
}
-
- if (!ReplacementsToMake.empty()) {
- LLVM_DEBUG(dbgs() << "Applying " << ReplacementsToMake.size()
- << " XOR->OR Disjoint replacements in " << F.getName()
- << "\n");
- for (auto &Pair : ReplacementsToMake)
- Pair.first->replaceAllUsesWith(Pair.second);
-
- for (auto &Pair : ReplacementsToMake)
- Pair.first->eraseFromParent();
- }
-
- return FunctionChanged;
+ return false;
}
-static llvm::Instruction *findClosestSequentialXor(Value *A, Instruction &I) {
- llvm::Instruction *ClosestUser = nullptr;
- for (llvm::User *User : A->users()) {
- if (auto *UserInst = llvm::dyn_cast<llvm::Instruction>(User)) {
- if (UserInst->getOpcode() != Instruction::Xor || UserInst == &I)
- continue;
- if (!ClosestUser)
- ClosestUser = UserInst;
- else {
- // Compare instruction positions.
- if (UserInst->comesBefore(ClosestUser)) {
- ClosestUser = UserInst;
- }
- }
- }
- }
- return ClosestUser;
-}
+bool XorToOrDisjointTransformer::processXorGroup(Value *OriginalBaseVal,
+ XorOpList &XorsInGroup) {
+ bool Changed = false;
+ if (XorsInGroup.size() <= 1)
+ return false;
-/// Try to transform I = xor(A, C1) into or_disjoint(Y, C2)
-/// where Y = xor(A, C0) is another existing instruction dominating I,
-/// C2 = C1 - C0, and A is known to be disjoint with C2.
-///
-/// This transformation is beneficial particularly for GEPs because:
-/// 1. OR operations often map better to addressing modes than XOR
-/// 2. Disjoint OR operations preserve the semantics of the original XOR
-/// 3. This can enable further optimizations in the GEP offset folding pipeline
-///
-/// @param I The XOR instruction being visited.
-/// @return The replacement Value* if successful, nullptr otherwise.
-Value *SeparateConstOffsetFromGEP::tryFoldXorToOrDisjoint(Instruction &I) {
- assert(I.getOpcode() == Instruction::Xor && "Instruction must be XOR");
-
- // Check if I has at least one GEP user.
- bool HasGepUser = false;
- for (User *U : I.users()) {
- if (isa<GetElementPtrInst>(U)) {
- HasGepUser = true;
+ // Sort XorsInGroup by the constant offset value in increasing order.
+ llvm::sort(
+ XorsInGroup.begin(), XorsInGroup.end(),
+ [](const auto &A, const auto &B) { return A.second.ult(B.second); });
+
+ // Dominance check
+ // The "base" XOR for dominance purposes is the one with the smallest
+ // constant.
+ BinaryOperator *XorWithSmallConst = XorsInGroup[0].first;
+
+ for (size_t i = 1; i < XorsInGroup.size(); ++i) {
+ BinaryOperator *currentXorToProcess = XorsInGroup[i].first;
+
+ // Check if the XorWithSmallConst dominates currentXorToProcess.
+ // If not, clone and add the instruction.
+ if (!DT.dominates(XorWithSmallConst, currentXorToProcess)) {
+ LLVM_DEBUG(
+ dbgs() << DEBUG_TYPE
+ << ": Cloning and inserting XOR with smallest constant ("
+ << *XorWithSmallConst << ") as it does not dominate "
+ << *currentXorToProcess << " in function " << F.getName()
+ << "\n");
+
+ BinaryOperator *ClonedXor =
+ cast<BinaryOperator>(XorWithSmallConst->clone());
+ ClonedXor->setName(XorWithSmallConst->getName() + ".dom_clone");
+ ClonedXor->insertAfter(dyn_cast<Instruction>(OriginalBaseVal));
+ LLVM_DEBUG(dbgs() << " Cloned Inst: " << *ClonedXor << "\n");
+ Changed = true;
+ XorWithSmallConst = ClonedXor;
break;
}
}
- // If no user is a GEP instruction, abort the transformation.
- if (!HasGepUser) {
- LLVM_DEBUG(
- dbgs() << "SeparateConstOffsetFromGEP: Skipping XOR->OR DISJOINT for"
- << I << " because it has no GEP users.\n");
- return nullptr;
- }
- Value *Op0 = I.getOperand(0);
- Value *Op1 = I.getOperand(1);
- ConstantInt *C1 = dyn_cast<ConstantInt>(Op1);
- Value *A = Op0;
-
- // Bail out of there is not constant operand.
- if (!C1) {
- C1 = dyn_cast<ConstantInt>(Op0);
- if (!C1)
- return nullptr;
- A = Op1;
- }
+ SmallVector<Instruction *, 8> InstructionsToErase;
+ const APInt SmallestConst =
+ dyn_cast<ConstantInt>(XorWithSmallConst->getOperand(1))->getValue();
- if (isa<UndefValue>(A))
- return nullptr;
+ // Main transformation loop: Iterate over the original XORs in the sorted
+ // group.
+ for (const auto &XorEntry : XorsInGroup) {
+ BinaryOperator *XorInst = XorEntry.first; // Original XOR instruction
+ const APInt ConstOffsetVal = XorEntry.second;
- APInt C1_APInt = C1->getValue();
- unsigned BitWidth = C1_APInt.getBitWidth();
- Type *Ty = I.getType();
+ // Do not process the one with smallest constant as it is the base.
+ if (XorInst == XorWithSmallConst)
+ continue;
- // Find Dominating Y = xor A, C0
- Instruction *FoundUserInst = nullptr;
- APInt C0_APInt;
+ // Disjointness Check 1
+ APInt NewConstVal = ConstOffsetVal - SmallestConst;
+ if ((NewConstVal & SmallestConst) != 0) {
+ LLVM_DEBUG(dbgs() << DEBUG_TYPE << ": Cannot transform XOR in function "
+ << F.getName() << ":\n"
+ << " New Const: " << NewConstVal << "\n"
+ << " Smallest Const: " << SmallestConst << "\n"
+ << " are not disjoint \n");
+ continue;
+ }
- // Find the closest XOR instruction using the same value.
- Instruction *UserInst = findClosestSequentialXor(A, I);
- if (!UserInst) {
- LLVM_DEBUG(
- dbgs() << "SeparateConstOffsetFromGEP: No dominating XOR found for" << I
- << "\n");
- return nullptr;
+ // Disjointness Check 2
+ KnownBits KnownBaseBits(
+ XorWithSmallConst->getType()->getScalarSizeInBits());
+ computeKnownBits(XorWithSmallConst, KnownBaseBits, DL, 0, nullptr,
+ XorWithSmallConst, &DT);
+ if ((KnownBaseBits.Zero & NewConstVal) == NewConstVal) {
+ LLVM_DEBUG(dbgs() << DEBUG_TYPE
+ << ": Transforming XOR to OR (disjoint) in function "
+ << F.getName() << ":\n"
+ << " Xor: " << *XorInst << "\n"
+ << " Base Val: " << *XorWithSmallConst << "\n"
+ << " New Const: " << NewConstVal << "\n");
+
+ auto *NewOrInst = BinaryOperator::CreateDisjointOr(
+ XorWithSmallConst,
+ ConstantInt::get(OriginalBaseVal->getType(), NewConstVal),
+ XorInst->getName() + ".or_disjoint", XorInst->getIterator());
+
+ NewOrInst->copyMetadata(*XorInst);
+ XorInst->replaceAllUsesWith(NewOrInst);
+ LLVM_DEBUG(dbgs() << " New Inst: " << *NewOrInst << "\n");
+ InstructionsToErase.push_back(XorInst); // Mark original XOR for deletion
+
+ Changed = true;
+ } else {
+ LLVM_DEBUG(
+ dbgs() << DEBUG_TYPE
+ << ": Cannot transform XOR (not proven disjoint) in function "
+ << F.getName() << ":\n"
+ << " Xor: " << *XorInst << "\n"
+ << " Base Val: " << *XorWithSmallConst << "\n"
+ << " New Const: " << NewConstVal << "\n");
+ }
}
+ if (!InstructionsToErase.empty())
+ for (Instruction *I : InstructionsToErase)
+ I->eraseFromParent();
- BinaryOperator *UserBO = cast<BinaryOperator>(UserInst);
- Value *UserOp0 = UserBO->getOperand(0);
- Value *UserOp1 = UserBO->getOperand(1);
- ConstantInt *UserC = nullptr;
- if (UserOp0 == A)
- UserC = dyn_cast<ConstantInt>(UserOp1);
- else if (UserOp1 == A)
- UserC = dyn_cast<ConstantInt>(UserOp0);
- else {
- LLVM_DEBUG(dbgs() << "SeparateConstOffsetFromGEP: Found XOR" << *UserInst
- << " doesn't use value " << *A << "\n");
- return nullptr;
- }
+ return Changed;
+}
- if (!UserC) {
- LLVM_DEBUG(
- dbgs()
- << "SeparateConstOffsetFromGEP: Found XOR doesn't have constant operand"
- << *UserInst << "\n");
- return nullptr;
- }
+// Try to transform XOR(A, B+C) in to XOR(A,C) + B where XOR(A,C) becomes
+// the base for memory operations. This transformation is true under the
+// following conditions
+// Check 1 - B and C are disjoint.
+// Check 2 - XOR(A,C) and B are disjoint.
+//
+// This transformation is beneficial particularly for GEPs because:
+// 1. OR operations often map better to addressing modes than XOR
+// 2. Disjoint OR operations preserve the semantics of the original XOR
+// 3. This can enable further optimizations in the GEP offset folding pipeline
+bool XorToOrDisjointTransformer::run() {
+ bool Changed = false;
- if (!DT->dominates(UserInst, &I)) {
- LLVM_DEBUG(dbgs() << "SeparateConstOffsetFromGEP: Found XOR" << *UserInst
- << " doesn't dominate " << I << "\n");
- return nullptr;
+ // Collect all candidate XORs
+ for (Instruction &I : instructions(F)) {
+ if (auto *XorOp = dyn_cast<BinaryOperator>(&I)) {
+ if (XorOp->getOpcode() == Instruction::Xor) {
+ Value *Op0 = XorOp->getOperand(0);
+ ConstantInt *C1 = nullptr;
+ // Match: xor Op0, Constant
+ if (match(XorOp->getOperand(1), m_ConstantInt(C1))) {
+ if (hasGEPUser(XorOp)) {
+ XorGroups[Op0].push_back({XorOp, C1->getValue()});
+ }
+ }
+ }
+ }
}
- FoundUserInst = UserInst;
- C0_APInt = UserC->getValue();
-
- // Calculate C2 = C1 - C0.
- APInt C2_APInt = C1_APInt - C0_APInt;
-
- // Check Disjointness A & C2 == 0.
- KnownBits KnownA(BitWidth);
- computeKnownBits(A, KnownA, *DL, 0, nullptr, &I, DT);
+ if (XorGroups.empty())
+ return false;
- if ((KnownA.One & C2_APInt) != 0) {
- LLVM_DEBUG(
- dbgs() << "SeparateConstOffsetFromGEP: Disjointness check failed for"
- << I << "\n");
- return nullptr;
+ // Process each group of XORs
+ for (auto &GroupPair : XorGroups) {
+ Value *OriginalBaseVal = GroupPair.first;
+ XorOpList &XorsInGroup = GroupPair.second;
+ if (processXorGroup(OriginalBaseVal, XorsInGroup))
+ Changed = true;
}
- IRBuilder<> Builder(&I);
- Builder.SetInsertPoint(&I);
- Constant *C2_Const = ConstantInt::get(Ty, C2_APInt);
- Twine Name = I.getName();
- Value *NewOr = BinaryOperator::CreateDisjointOr(FoundUserInst, C2_Const, Name,
- I.getIterator());
- // Preserve metadata
- if (Instruction *NewOrInst = dyn_cast<Instruction>(NewOr))
- NewOrInst->copyMetadata(I);
-
- LLVM_DEBUG(dbgs() << "SeparateConstOffsetFromGEP: Replacing" << I
- << " (used by GEP) with" << *NewOr << " based on"
- << *FoundUserInst << "\n");
-
- return NewOr;
+ return Changed;
}
bool SeparateConstOffsetFromGEPLegacyPass::runOnFunction(Function &F) {
@@ -1361,7 +1382,8 @@ bool SeparateConstOffsetFromGEP::run(Function &F) {
bool Changed = false;
// Decompose xor in to "or disjoint" if possible.
- Changed |= decomposeXor(F);
+ XorToOrDisjointTransformer XorTransformer(F, *DT, *DL);
+ Changed |= XorTransformer.run();
for (BasicBlock &B : F) {
if (!DT->isReachableFromEntry(&B))
diff --git a/llvm/test/Transforms/SeparateConstOffsetFromGEP/AMDGPU/xor-to-or-disjoint.ll b/llvm/test/Transforms/SeparateConstOffsetFromGEP/AMDGPU/xor-to-or-disjoint.ll
index d1a04a70d4994..cc51368363a42 100644
--- a/llvm/test/Transforms/SeparateConstOffsetFromGEP/AMDGPU/xor-to-or-disjoint.ll
+++ b/llvm/test/Transforms/SeparateConstOffsetFromGEP/AMDGPU/xor-to-or-disjoint.ll
@@ -3,142 +3,152 @@
; RUN: -S < %s | FileCheck %s
-; Test with GEP user and known bits: Ensure the transformation occurs when the xor has a GEP user
-define ptr @test_with_gep_user(ptr %ptr) {
-; CHECK-LABEL: define ptr @test_with_gep_user(
-; CHECK-SAME: ptr [[PTR:%.*]]) {
+; Test a simple case of xor to or disjoint transformation
+define void @test_basic_transformation(ptr %ptr, i64 %input) {
+; CHECK-LABEL: define void @test_basic_transformation(
+; CHECK-SAME: ptr [[PTR:%.*]], i64 [[INPUT:%.*]]) {
; CHECK-NEXT: [[ENTRY:.*:]]
-; CHECK-NEXT: [[BASE:%.*]] = add i64 0, 0
-; CHECK-NEXT: [[XOR1:%.*]] = xor i64 [[BASE]], 8
-; CHECK-NEXT: [[XOR21:%.*]] = or disjoint i64 [[XOR1]], 16
-; CHECK-NEXT: [[GEP:%.*]] = getelementptr i8, ptr [[PTR]], i64 [[XOR21]]
-; CHECK-NEXT: ret ptr [[GEP]]
+; CHECK-NEXT: [[BASE:%.*]] = and i64 [[INPUT]], -8192
+; CHECK-NEXT: [[ADDR1:%.*]] = xor i64 [[BASE]], 32
+; CHECK-NEXT: [[ADDR2_OR_DISJOINT:%.*]] = or disjoint i64 [[ADDR1]], 2048
+; CHECK-NEXT: [[ADDR3_OR_DISJOINT:%.*]] = or disjoint i64 [[ADDR1]], 4096
+; CHECK-NEXT: [[GEP1:%.*]] = getelementptr i8, ptr [[PTR]], i64 [[ADDR1]]
+; CHECK-NEXT: [[GEP2:%.*]] = getelementptr i8, ptr [[PTR]], i64 [[ADDR2_OR_DISJOINT]]
+; CHECK-NEXT: [[GEP3:%.*]] = getelementptr i8, ptr [[PTR]], i64 [[ADDR3_OR_DISJOINT]]
+; CHECK-NEXT: [[VAL1:%.*]] = load half, ptr [[GEP1]], align 2
+; CHECK-NEXT: [[VAL2:%.*]] = load half, ptr [[GEP2]], align 2
+; CHECK-NEXT: [[VAL3:%.*]] = load half, ptr [[GEP3]], align 2
+; CHECK-NEXT: ret void
;
entry:
- %base = add i64 0,0
- %xor1 = xor i64 %base, 8
- %xor2 = xor i64 %base, 24 ; Should be replaced with OR of %xor1 and 16
- %gep = getelementptr i8, ptr %ptr, i64 %xor2
- ret ptr %gep
+ %base = and i64 %input, -8192 ; Clear low bits
+ %addr1 = xor i64 %base, 32
+ %addr2 = xor i64 %base, 2080
+ %addr3 = xor i64 %base, 4128
+ %gep1 = getelementptr i8, ptr %ptr, i64 %addr1
+ %gep2 = getelementptr i8, ptr %ptr, i64 %addr2
+ %gep3 = getelementptr i8, ptr %ptr, i64 %addr3
+ %val1 = load half, ptr %gep1
+ %val2 = load half, ptr %gep2
+ %val3 = load half, ptr %gep3
+ ret void
}
-; Test with non-GEP user: Ensure the transformation does not occur
-define i32 @test_with_non_gep_user(ptr %ptr) {
-; CHECK-LABEL: define i32 @test_with_non_gep_user(
-; CHECK-SAME: ptr [[PTR:%.*]]) {
+; Test the decreasing order of offset xor to or disjoint transformation
+define void @test_descending_offset_transformation(ptr %ptr, i64 %input) {
+; CHECK-LABEL: define void @test_descending_offset_transformation(
+; CHECK-SAME: ptr [[PTR:%.*]], i64 [[INPUT:%.*]]) {
; CHECK-NEXT: [[ENTRY:.*:]]
-; CHECK-NEXT: [[BASE:%.*]] = add i32 0, 0
-; CHECK-NEXT: [[XOR1:%.*]] = xor i32 [[BASE]], 8
-; CHECK-NEXT: [[XOR2:%.*]] = xor i32 [[BASE]], 24
-; CHECK-NEXT: [[ADD:%.*]] = add i32 [[XOR2]], 5
-; CHECK-NEXT: ret i32 [[ADD]]
+; CHECK-NEXT: [[BASE:%.*]] = and i64 [[INPUT]], -8192
+; CHECK-NEXT: [[ADDR3_DOM_CLONE:%.*]] = xor i64 [[BASE]], 32
+; CHECK-NEXT: [[ADDR1_OR_DISJOINT:%.*]] = or disjoint i64 [[ADDR3_DOM_CLONE]], 4096
+; CHECK-NEXT: [[ADDR2_OR_DISJOINT:%.*]] = or disjoint i64 [[ADDR3_DOM_CLONE]], 2048
+; CHECK-NEXT: [[ADDR3_OR_DISJOINT:%.*]] = or disjoint i64 [[ADDR3_DOM_CLONE]], 0
+; CHECK-NEXT: [[GEP1:%.*]] = getelementptr i8, ptr [[PTR]], i64 [[ADDR1_OR_DISJOINT]]
+; CHECK-NEXT: [[GEP2:%.*]] = getelementptr i8, ptr [[PTR]], i64 [[ADDR2_OR_DISJOINT]]
+; CHECK-NEXT: [[GEP3:%.*]] = getelementptr i8, ptr [[PTR]], i64 [[ADDR3_OR_DISJOINT]]
+; CHECK-NEXT: [[VAL1:%.*]] = load half, ptr [[GEP1]], align 2
+; CHECK-NEXT: [[VAL2:%.*]] = load half, ptr [[GEP2]], align 2
+; CHECK-NEXT: [[VAL3:%.*]] = load half, ptr [[GEP3]], align 2
+; CHECK-NEXT: ret void
;
entry:
- %base = add i32 0,0
- %xor1 = xor i32 %base, 8
- %xor2 = xor i32 %base, 24
- %add = add i32 %xor2, 5
- ret i32 %add
+ %base = and i64 %input, -8192 ; Clear low bits
+ %addr1 = xor i64 %base, 4128
+ %addr2 = xor i64 %base, 2080
+ %addr3 = xor i64 %base, 32
+ %gep1 = getelementptr i8, ptr %ptr, i64 %addr1
+ %gep2 = getelementptr i8, ptr %ptr, i64 %addr2
+ %gep3 = getelementptr i8, ptr %ptr, i64 %addr3
+ %val1 = load half, ptr %gep1
+ %val2 = load half, ptr %gep2
+ %val3 = load half, ptr %gep3
+ ret void
}
-; Test with non-constant operand: Ensure the transformation does not occur
-define ptr @test_with_non_constant_operand(i64 %val, i64 %val2, ptr %ptr) {
-; CHECK-LABEL: define ptr @test_with_non_constant_operand(
-; CHECK-SAME: i64 [[VAL:%.*]], i64 [[VAL2:%.*]], ptr [[PTR:%.*]]) {
-; CHECK-NEXT: [[ENTRY:.*:]]
-; CHECK-NEXT: [[XOR1:%.*]] = xor i64 [[VAL]], [[VAL2]]
-; CHECK-NEXT: [[XOR2:%.*]] = xor i64 [[VAL]], 24
-; CHECK-NEXT: [[GEP:%.*]] = getelementptr i8, ptr [[PTR]], i64 [[XOR2]]
-; CHECK-NEXT: ret ptr [[GEP]]
-;
-entry:
- %xor1 = xor i64 %val, %val2 ; Non-constant operand
- %xor2 = xor i64 %val, 24
- %gep = getelementptr i8, ptr %ptr, i64 %xor2
- ret ptr %gep
-}
-; Test with unknown disjoint bits: Ensure the transformation does not occur
-define ptr @test_with_unknown_disjoint_bits(i64 %base, ptr %ptr) {
-; CHECK-LABEL: define ptr @test_with_unknown_disjoint_bits(
-; CHECK-SAME: i64 [[BASE:%.*]], ptr [[PTR:%.*]]) {
+; Test that %addr2 is not transformed to or disjoint.
+define void @test_no_transfomation(ptr %ptr, i64 %input) {
+; CHECK-LABEL: define void @test_no_transfomation(
+; CHECK-SAME: ptr [[PTR:%.*]], i64 [[INPUT:%.*]]) {
; CHECK-NEXT: [[ENTRY:.*:]]
-; CHECK-NEXT: [[XOR1:%.*]] = xor i64 [[BASE]], 8
-; CHECK-NEXT: [[XOR21:%.*]] = or disjoint i64 [[XOR1]], 16
-; CHECK-NEXT: [[GEP:%.*]] = getelementptr i8, ptr [[PTR]], i64 [[XOR21]]
-; CHECK-NEXT: ret ptr [[GEP]]
+; CHECK-NEXT: [[BASE:%.*]] = and i64 [[INPUT]], -8192
+; CHECK-NEXT: [[ADDR1:%.*]] = xor i64 [[BASE]], 32
+; CHECK-NEXT: [[ADDR2:%.*]] = xor i64 [[BASE]], 64
+; CHECK-NEXT: [[ADDR3_OR_DISJOINT:%.*]] = or disjoint i64 [[ADDR1]], 2048
+; CHECK-NEXT: [[GEP1:%.*]] = getelementptr i8, ptr [[PTR]], i64 [[ADDR1]]
+; CHECK-NEXT: [[GEP2:%.*]] = getelementptr i8, ptr [[PTR]], i64 [[ADDR2]]
+; CHECK-NEXT: [[GEP3:%.*]] = getelementptr i8, ptr [[PTR]], i64 [[ADDR3_OR_DISJOINT]]
+; CHECK-NEXT: [[VAL1:%.*]] = load half, ptr [[GEP1]], align 2
+; CHECK-NEXT: [[VAL2:%.*]] = load half, ptr [[GEP2]], align 2
+; CHECK-NEXT: [[VAL3:%.*]] = load half, ptr [[GEP3]], align 2
+; CHECK-NEXT: ret void
;
entry:
- %xor1 = xor i64 %base, 8
- %xor2 = xor i64 %base, 24
- %gep = getelementptr i8, ptr %ptr, i64 %xor2
- ret ptr %gep
+ %base = and i64 %input, -8192 ; Clear low bits
+ %addr1 = xor i64 %base, 32
+ %addr2 = xor i64 %base, 64 ; Should not be transformed
+ %addr3 = xor i64 %base, 2080
+ %gep1 = getelementptr i8, ptr %ptr, i64 %addr1
+ %gep2 = getelementptr i8, ptr %ptr, i64 %addr2
+ %gep3 = getelementptr i8, ptr %ptr, i64 %addr3
+ %val1 = load half, ptr %gep1
+ %val2 = load half, ptr %gep2
+ %val3 = load half, ptr %gep3
+ ret void
}
-; Test with multiple xor operations in sequence
-define ptr @test_multiple_xors(ptr %ptr) {
-; CHECK-LABEL: define ptr @test_multiple_xors(
-; CHECK-SAME: ptr [[PTR:%.*]]) {
-; CHECK-NEXT: [[ENTRY:.*:]]
-; CHECK-NEXT: [[BASE:%.*]] = add i64 2, 0
-; CHECK-NEXT: [[XOR1:%.*]] = xor i64 [[BASE]], 8
-; CHECK-NEXT: [[XOR21:%.*]] = or disjoint i64 [[XOR1]], 16
-; CHECK-NEXT: [[XOR32:%.*]] = or disjoint i64 [[XOR1]], 24
-; CHECK-NEXT: [[XOR43:%.*]] = or disjoint i64 [[XOR1]], 64
-; CHECK-NEXT: [[GEP2:%.*]] = getelementptr i8, ptr [[PTR]], i64 [[XOR21]]
-; CHECK-NEXT: [[GEP3:%.*]] = getelementptr i8, ptr [[PTR]], i64 [[XOR32]]
-; CHECK-NEXT: [[GEP4:%.*]] = getelementptr i8, ptr [[PTR]], i64 [[XOR43]]
-; CHECK-NEXT: ret ptr [[GEP4]]
-;
-entry:
- %base = add i64 2,0
- %xor1 = xor i64 %base, 8
- %xor2 = xor i64 %base, 24 ; Should be replaced with OR
- %xor3 = xor i64 %base, 32
- %xor4 = xor i64 %base, 72 ; Should be replaced with OR
- %gep2 = getelementptr i8, ptr %ptr, i64 %xor2
- %gep3 = getelementptr i8, ptr %ptr, i64 %xor3
- %gep4 = getelementptr i8, ptr %ptr, i64 %xor4
- ret ptr %gep4
-}
-
-; Test with operand order variations
-define ptr @test_operand_order(ptr %ptr) {
-; CHECK-LABEL: define ptr @test_operand_order(
-; CHECK-SAME: ptr [[PTR:%.*]]) {
+; Test case with xor instructions in different basic blocks
+define void @test_dom_tree(ptr %ptr, i64 %input, i1 %cond) {
+; CHECK-LABEL: define void @test_dom_tree(
+; CHECK-SAME: ptr [[PTR:%.*]], i64 [[INPUT:%.*]], i1 [[COND:%.*]]) {
; CHECK-NEXT: [[ENTRY:.*:]]
-; CHECK-NEXT: [[BASE:%.*]] = add i64 2, 0
-; CHECK-NEXT: [[XOR1:%.*]] = xor i64 [[BASE]], 12
-; CHECK-NEXT: [[XOR21:%.*]] = or disjoint i64 [[XOR1]], 12
-; CHECK-NEXT: [[GEP:%.*]] = getelementptr i8, ptr [[PTR]], i64 [[XOR21]]
-; CHECK-NEXT: ret ptr [[GEP]]
+; CHECK-NEXT: [[BASE:%.*]] = and i64 [[INPUT]], -8192
+; CHECK-NEXT: [[ADDR1:%.*]] = xor i64 [[BASE]], 16
+; CHECK-NEXT: [[GEP1:%.*]] = getelementptr i8, ptr [[PTR]], i64 [[ADDR1]]
+; CHECK-NEXT: [[VAL1:%.*]] = load half, ptr [[GEP1]], align 2
+; CHECK-NEXT: br i1 [[COND]], label %[[THEN:.*]], label %[[ELSE:.*]]
+; CHECK: [[THEN]]:
+; CHECK-NEXT: [[ADDR2_OR_DISJOINT:%.*]] = or disjoint i64 [[ADDR1]], 32
+; CHECK-NEXT: [[GEP2:%.*]] = getelementptr i8, ptr [[PTR]], i64 [[ADDR2_OR_DISJOINT]]
+; CHECK-NEXT: [[VAL2:%.*]] = load half, ptr [[GEP2]], align 2
+; CHECK-NEXT: br label %[[MERGE:.*]]
+; CHECK: [[ELSE]]:
+; CHECK-NEXT: [[ADDR3_OR_DISJOINT:%.*]] = or disjoint i64 [[ADDR1]], 96
+; CHECK-NEXT: [[GEP3:%.*]] = getelementptr i8, ptr [[PTR]], i64 [[ADDR3_OR_DISJOINT]]
+; CHECK-NEXT: [[VAL3:%.*]] = load half, ptr [[GEP3]], align 2
+; CHECK-NEXT: br label %[[MERGE]]
+; CHECK: [[MERGE]]:
+; CHECK-NEXT: [[ADDR4_OR_DISJOINT:%.*]] = or disjoint i64 [[ADDR1]], 224
+; CHECK-NEXT: [[GEP4:%.*]] = getelementptr i8, ptr [[PTR]], i64 [[ADDR4_OR_DISJOINT]]
+; CHECK-NEXT: [[VAL4:%.*]] = load half, ptr [[GEP4]], align 2
+; CHECK-NEXT: ret void
;
entry:
- %base = add i64 2,0
- %xor1 = xor i64 %base, 12
- %xor2 = xor i64 24, %base ; Operands reversed, should still be replaced
- %gep = getelementptr i8, ptr %ptr, i64 %xor2
- ret ptr %gep
-}
+ %base = and i64 %input, -8192 ; Clear low bits
+ %addr1 = xor i64 %base,16
+ %gep1 = getelementptr i8, ptr %ptr, i64 %addr1
+ %val1 = load half, ptr %gep1
+ br i1 %cond, label %then, label %else
+then:
+ %addr2 = xor i64 %base, 48
+ %gep2 = getelementptr i8, ptr %ptr, i64 %addr2
+ %val2 = load half, ptr %gep2
+ br label %merge
-; Test with multiple xor operations in sequence
-define ptr @test_negative_offset(ptr %ptr) {
-; CHECK-LABEL: define ptr @test_negative_offset(
-; CHECK-SAME: ptr [[PTR:%.*]]) {
-; CHECK-NEXT: [[ENTRY:.*:]]
-; CHECK-NEXT: [[BASE:%.*]] = add i64 2, 0
-; CHECK-NEXT: [[XOR1:%.*]] = xor i64 [[BASE]], 72
-; CHECK-NEXT: [[XOR21:%.*]] = or disjoint i64 [[XOR1]], -48
-; CHECK-NEXT: [[GEP:%.*]] = getelementptr i8, ptr [[PTR]], i64 [[XOR21]]
-; CHECK-NEXT: ret ptr [[GEP]]
-;
-entry:
- %base = add i64 2,0
- %xor1 = xor i64 %base, 72
- %xor2 = xor i64 %base, 24 ; Should be replaced with OR
- %gep = getelementptr i8, ptr %ptr, i64 %xor2
- ret ptr %gep
+else:
+ %addr3 = xor i64 %base, 112
+ %gep3 = getelementptr i8, ptr %ptr, i64 %addr3
+ %val3 = load half, ptr %gep3
+ br label %merge
+
+merge:
+ %addr4 = xor i64 %base, 240
+ %gep4 = getelementptr i8, ptr %ptr, i64 %addr4
+ %val4 = load half, ptr %gep4
+ ret void
}
+
>From 1d56b8030ab71d77edc35a580b1b8ad170a06a0a Mon Sep 17 00:00:00 2001
From: Sumanth Gundapaneni <sugundap at amd.com>
Date: Tue, 13 May 2025 14:14:02 -0500
Subject: [PATCH 5/8] Removed dead code
---
llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp | 3 ---
1 file changed, 3 deletions(-)
diff --git a/llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp b/llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp
index eb1d85584c0f6..cb2aecd9b2855 100644
--- a/llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp
+++ b/llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp
@@ -488,9 +488,6 @@ class SeparateConstOffsetFromGEP {
DenseMap<ExprKey, SmallVector<Instruction *, 2>> DominatingAdds;
DenseMap<ExprKey, SmallVector<Instruction *, 2>> DominatingSubs;
-
- bool decomposeXor(Function &F);
- Value *tryFoldXorToOrDisjoint(Instruction &I);
};
/// A helper class that aims to convert xor operations into or operations when
>From 0f3c655c1198c872dc6de634e7fb1111e71381fa Mon Sep 17 00:00:00 2001
From: Sumanth Gundapaneni <sugundap at amd.com>
Date: Tue, 20 May 2025 09:46:26 -0500
Subject: [PATCH 6/8] Address comments
---
.../Scalar/SeparateConstOffsetFromGEP.cpp | 34 ++++++++++---------
1 file changed, 18 insertions(+), 16 deletions(-)
diff --git a/llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp b/llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp
index cb2aecd9b2855..75e4acd204a93 100644
--- a/llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp
+++ b/llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp
@@ -511,16 +511,16 @@ class XorToOrDisjointTransformer {
const DataLayout &DL;
/// Maps a common operand to all Xor instructions
using XorOpList = SmallVector<std::pair<BinaryOperator *, APInt>, 8>;
- using XorBaseValMap = DenseMap<Value *, XorOpList>;
- XorBaseValMap XorGroups;
+ using XorBaseValInst = DenseMap<Instruction *, XorOpList>;
+ XorBaseValInst XorGroups;
/// Checks if the given value has at least one GetElementPtr user
- bool hasGEPUser(const Value *V) const;
+ static bool hasGEPUser(const Value *V);
/// Processes a group of XOR instructions that share the same non-constant
/// base operand. Returns true if this group's processing modified the
/// function.
- bool processXorGroup(Value *OriginalBaseVal, XorOpList &XorsInGroup);
+ bool processXorGroup(Instruction *OriginalBaseInst, XorOpList &XorsInGroup);
};
} // end anonymous namespace
@@ -1200,7 +1200,7 @@ bool SeparateConstOffsetFromGEP::splitGEP(GetElementPtrInst *GEP) {
}
// Helper function to check if an instruction has at least one GEP user
-bool XorToOrDisjointTransformer::hasGEPUser(const Value *V) const {
+bool XorToOrDisjointTransformer::hasGEPUser(const Value *V) {
for (const User *U : V->users()) {
if (isa<GetElementPtrInst>(U)) {
return true;
@@ -1209,7 +1209,7 @@ bool XorToOrDisjointTransformer::hasGEPUser(const Value *V) const {
return false;
}
-bool XorToOrDisjointTransformer::processXorGroup(Value *OriginalBaseVal,
+bool XorToOrDisjointTransformer::processXorGroup(Instruction *OriginalBaseInst,
XorOpList &XorsInGroup) {
bool Changed = false;
if (XorsInGroup.size() <= 1)
@@ -1241,7 +1241,7 @@ bool XorToOrDisjointTransformer::processXorGroup(Value *OriginalBaseVal,
BinaryOperator *ClonedXor =
cast<BinaryOperator>(XorWithSmallConst->clone());
ClonedXor->setName(XorWithSmallConst->getName() + ".dom_clone");
- ClonedXor->insertAfter(dyn_cast<Instruction>(OriginalBaseVal));
+ ClonedXor->insertAfter(OriginalBaseInst);
LLVM_DEBUG(dbgs() << " Cloned Inst: " << *ClonedXor << "\n");
Changed = true;
XorWithSmallConst = ClonedXor;
@@ -1251,7 +1251,7 @@ bool XorToOrDisjointTransformer::processXorGroup(Value *OriginalBaseVal,
SmallVector<Instruction *, 8> InstructionsToErase;
const APInt SmallestConst =
- dyn_cast<ConstantInt>(XorWithSmallConst->getOperand(1))->getValue();
+ cast<ConstantInt>(XorWithSmallConst->getOperand(1))->getValue();
// Main transformation loop: Iterate over the original XORs in the sorted
// group.
@@ -1289,7 +1289,7 @@ bool XorToOrDisjointTransformer::processXorGroup(Value *OriginalBaseVal,
auto *NewOrInst = BinaryOperator::CreateDisjointOr(
XorWithSmallConst,
- ConstantInt::get(OriginalBaseVal->getType(), NewConstVal),
+ ConstantInt::get(OriginalBaseInst->getType(), NewConstVal),
XorInst->getName() + ".or_disjoint", XorInst->getIterator());
NewOrInst->copyMetadata(*XorInst);
@@ -1308,9 +1308,9 @@ bool XorToOrDisjointTransformer::processXorGroup(Value *OriginalBaseVal,
<< " New Const: " << NewConstVal << "\n");
}
}
- if (!InstructionsToErase.empty())
- for (Instruction *I : InstructionsToErase)
- I->eraseFromParent();
+
+ for (Instruction *I : InstructionsToErase)
+ I->eraseFromParent();
return Changed;
}
@@ -1335,9 +1335,11 @@ bool XorToOrDisjointTransformer::run() {
Value *Op0 = XorOp->getOperand(0);
ConstantInt *C1 = nullptr;
// Match: xor Op0, Constant
- if (match(XorOp->getOperand(1), m_ConstantInt(C1))) {
+ if (isa<Instruction>(Op0) &&
+ match(XorOp->getOperand(1), m_ConstantInt(C1))) {
if (hasGEPUser(XorOp)) {
- XorGroups[Op0].push_back({XorOp, C1->getValue()});
+ Instruction *InstOp0 = cast<Instruction>(Op0);
+ XorGroups[InstOp0].push_back({XorOp, C1->getValue()});
}
}
}
@@ -1349,9 +1351,9 @@ bool XorToOrDisjointTransformer::run() {
// Process each group of XORs
for (auto &GroupPair : XorGroups) {
- Value *OriginalBaseVal = GroupPair.first;
+ Instruction *OriginalBaseInst = cast<Instruction>(GroupPair.first);
XorOpList &XorsInGroup = GroupPair.second;
- if (processXorGroup(OriginalBaseVal, XorsInGroup))
+ if (processXorGroup(OriginalBaseInst, XorsInGroup))
Changed = true;
}
>From 673dd1b6ed14def2b6b590125edbe8bd3369b732 Mon Sep 17 00:00:00 2001
From: Sumanth Gundapaneni <sugundap at amd.com>
Date: Wed, 21 May 2025 17:28:21 -0500
Subject: [PATCH 7/8] Address reviewer comments, take 2
---
.../Scalar/SeparateConstOffsetFromGEP.cpp | 75 ++++++++++---------
1 file changed, 38 insertions(+), 37 deletions(-)
diff --git a/llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp b/llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp
index 75e4acd204a93..dcada218be39a 100644
--- a/llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp
+++ b/llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp
@@ -517,6 +517,9 @@ class XorToOrDisjointTransformer {
/// Checks if the given value has at least one GetElementPtr user
static bool hasGEPUser(const Value *V);
+ /// Helper function to check if BaseXor dominates all XORs in the group
+ bool dominatesAllXors(BinaryOperator *BaseXor, const XorOpList &XorsInGroup);
+
/// Processes a group of XOR instructions that share the same non-constant
/// base operand. Returns true if this group's processing modified the
/// function.
@@ -1209,6 +1212,17 @@ bool XorToOrDisjointTransformer::hasGEPUser(const Value *V) {
return false;
}
+bool XorToOrDisjointTransformer::dominatesAllXors(
+ BinaryOperator *BaseXor, const XorOpList &XorsInGroup) {
+ for (const auto &XorEntry : XorsInGroup) {
+ BinaryOperator *XorInst = XorEntry.first;
+ if (XorInst != BaseXor && !DT.dominates(BaseXor, XorInst)) {
+ return false;
+ }
+ }
+ return true;
+}
+
bool XorToOrDisjointTransformer::processXorGroup(Instruction *OriginalBaseInst,
XorOpList &XorsInGroup) {
bool Changed = false;
@@ -1225,28 +1239,20 @@ bool XorToOrDisjointTransformer::processXorGroup(Instruction *OriginalBaseInst,
// constant.
BinaryOperator *XorWithSmallConst = XorsInGroup[0].first;
- for (size_t i = 1; i < XorsInGroup.size(); ++i) {
- BinaryOperator *currentXorToProcess = XorsInGroup[i].first;
-
- // Check if the XorWithSmallConst dominates currentXorToProcess.
- // If not, clone and add the instruction.
- if (!DT.dominates(XorWithSmallConst, currentXorToProcess)) {
- LLVM_DEBUG(
- dbgs() << DEBUG_TYPE
- << ": Cloning and inserting XOR with smallest constant ("
- << *XorWithSmallConst << ") as it does not dominate "
- << *currentXorToProcess << " in function " << F.getName()
- << "\n");
-
- BinaryOperator *ClonedXor =
- cast<BinaryOperator>(XorWithSmallConst->clone());
- ClonedXor->setName(XorWithSmallConst->getName() + ".dom_clone");
- ClonedXor->insertAfter(OriginalBaseInst);
- LLVM_DEBUG(dbgs() << " Cloned Inst: " << *ClonedXor << "\n");
- Changed = true;
- XorWithSmallConst = ClonedXor;
- break;
- }
+ if (!dominatesAllXors(XorWithSmallConst, XorsInGroup)) {
+ LLVM_DEBUG(dbgs() << DEBUG_TYPE
+ << ": Cloning and inserting XOR with smallest constant ("
+ << *XorWithSmallConst
+ << ") as it does not dominate all other XORs"
+ << " in function " << F.getName() << "\n");
+
+ BinaryOperator *ClonedXor =
+ cast<BinaryOperator>(XorWithSmallConst->clone());
+ ClonedXor->setName(XorWithSmallConst->getName() + ".dom_clone");
+ ClonedXor->insertAfter(OriginalBaseInst);
+ LLVM_DEBUG(dbgs() << " Cloned Inst: " << *ClonedXor << "\n");
+ Changed = true;
+ XorWithSmallConst = ClonedXor;
}
SmallVector<Instruction *, 8> InstructionsToErase;
@@ -1330,20 +1336,15 @@ bool XorToOrDisjointTransformer::run() {
// Collect all candidate XORs
for (Instruction &I : instructions(F)) {
- if (auto *XorOp = dyn_cast<BinaryOperator>(&I)) {
- if (XorOp->getOpcode() == Instruction::Xor) {
- Value *Op0 = XorOp->getOperand(0);
- ConstantInt *C1 = nullptr;
- // Match: xor Op0, Constant
- if (isa<Instruction>(Op0) &&
- match(XorOp->getOperand(1), m_ConstantInt(C1))) {
- if (hasGEPUser(XorOp)) {
- Instruction *InstOp0 = cast<Instruction>(Op0);
- XorGroups[InstOp0].push_back({XorOp, C1->getValue()});
- }
- }
- }
- }
+ Instruction *Op0 = nullptr;
+ ConstantInt *C1 = nullptr;
+ BinaryOperator *MatchedXorOp = nullptr;
+
+ // Attempt to match the instruction 'I' as XOR operation.
+ if (match(&I, m_CombineAnd(m_Xor(m_Instruction(Op0), m_ConstantInt(C1)),
+ m_BinOp(MatchedXorOp))) &&
+ hasGEPUser(MatchedXorOp))
+ XorGroups[Op0].push_back({MatchedXorOp, C1->getValue()});
}
if (XorGroups.empty())
@@ -1351,7 +1352,7 @@ bool XorToOrDisjointTransformer::run() {
// Process each group of XORs
for (auto &GroupPair : XorGroups) {
- Instruction *OriginalBaseInst = cast<Instruction>(GroupPair.first);
+ Instruction *OriginalBaseInst = GroupPair.first;
XorOpList &XorsInGroup = GroupPair.second;
if (processXorGroup(OriginalBaseInst, XorsInGroup))
Changed = true;
>From 406a692ba0ed49addb3b646f2895d99026d74b19 Mon Sep 17 00:00:00 2001
From: Sumanth Gundapaneni <sugundap at amd.com>
Date: Thu, 29 May 2025 14:21:10 -0500
Subject: [PATCH 8/8] Address reviewer comments , take 3
---
.../Scalar/SeparateConstOffsetFromGEP.cpp | 43 ++++------
.../AMDGPU/xor-to-or-disjoint.ll | 82 +++++++++++++++----
2 files changed, 82 insertions(+), 43 deletions(-)
diff --git a/llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp b/llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp
index dcada218be39a..3109e1e1758d7 100644
--- a/llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp
+++ b/llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp
@@ -1204,23 +1204,18 @@ bool SeparateConstOffsetFromGEP::splitGEP(GetElementPtrInst *GEP) {
// Helper function to check if an instruction has at least one GEP user
bool XorToOrDisjointTransformer::hasGEPUser(const Value *V) {
- for (const User *U : V->users()) {
- if (isa<GetElementPtrInst>(U)) {
- return true;
- }
- }
- return false;
+ return llvm::any_of(V->users(), [](const User *U) {
+ return isa<llvm::GetElementPtrInst>(U);
+ });
}
bool XorToOrDisjointTransformer::dominatesAllXors(
BinaryOperator *BaseXor, const XorOpList &XorsInGroup) {
- for (const auto &XorEntry : XorsInGroup) {
+ return llvm::all_of(XorsInGroup, [&](const auto &XorEntry) {
BinaryOperator *XorInst = XorEntry.first;
- if (XorInst != BaseXor && !DT.dominates(BaseXor, XorInst)) {
- return false;
- }
- }
- return true;
+ // Do not evaluate the BaseXor, otherwise we end up cloning it.
+ return XorInst == BaseXor || DT.dominates(BaseXor, XorInst);
+ });
}
bool XorToOrDisjointTransformer::processXorGroup(Instruction *OriginalBaseInst,
@@ -1230,9 +1225,9 @@ bool XorToOrDisjointTransformer::processXorGroup(Instruction *OriginalBaseInst,
return false;
// Sort XorsInGroup by the constant offset value in increasing order.
- llvm::sort(
- XorsInGroup.begin(), XorsInGroup.end(),
- [](const auto &A, const auto &B) { return A.second.ult(B.second); });
+ llvm::sort(XorsInGroup, [](const auto &A, const auto &B) {
+ return A.second.slt(B.second);
+ });
// Dominance check
// The "base" XOR for dominance purposes is the one with the smallest
@@ -1274,18 +1269,15 @@ bool XorToOrDisjointTransformer::processXorGroup(Instruction *OriginalBaseInst,
if ((NewConstVal & SmallestConst) != 0) {
LLVM_DEBUG(dbgs() << DEBUG_TYPE << ": Cannot transform XOR in function "
<< F.getName() << ":\n"
- << " New Const: " << NewConstVal << "\n"
- << " Smallest Const: " << SmallestConst << "\n"
+ << " New Const: " << NewConstVal
+ << " Smallest Const: " << SmallestConst
<< " are not disjoint \n");
continue;
}
// Disjointness Check 2
- KnownBits KnownBaseBits(
- XorWithSmallConst->getType()->getScalarSizeInBits());
- computeKnownBits(XorWithSmallConst, KnownBaseBits, DL, 0, nullptr,
- XorWithSmallConst, &DT);
- if ((KnownBaseBits.Zero & NewConstVal) == NewConstVal) {
+ if (MaskedValueIsZero(XorWithSmallConst, NewConstVal, SimplifyQuery(DL),
+ 0)) {
LLVM_DEBUG(dbgs() << DEBUG_TYPE
<< ": Transforming XOR to OR (disjoint) in function "
<< F.getName() << ":\n"
@@ -1344,19 +1336,16 @@ bool XorToOrDisjointTransformer::run() {
if (match(&I, m_CombineAnd(m_Xor(m_Instruction(Op0), m_ConstantInt(C1)),
m_BinOp(MatchedXorOp))) &&
hasGEPUser(MatchedXorOp))
- XorGroups[Op0].push_back({MatchedXorOp, C1->getValue()});
+ XorGroups[Op0].emplace_back(MatchedXorOp, C1->getValue());
}
if (XorGroups.empty())
return false;
// Process each group of XORs
- for (auto &GroupPair : XorGroups) {
- Instruction *OriginalBaseInst = GroupPair.first;
- XorOpList &XorsInGroup = GroupPair.second;
+ for (auto &[OriginalBaseInst, XorsInGroup] : XorGroups)
if (processXorGroup(OriginalBaseInst, XorsInGroup))
Changed = true;
- }
return Changed;
}
diff --git a/llvm/test/Transforms/SeparateConstOffsetFromGEP/AMDGPU/xor-to-or-disjoint.ll b/llvm/test/Transforms/SeparateConstOffsetFromGEP/AMDGPU/xor-to-or-disjoint.ll
index cc51368363a42..825227292fe14 100644
--- a/llvm/test/Transforms/SeparateConstOffsetFromGEP/AMDGPU/xor-to-or-disjoint.ll
+++ b/llvm/test/Transforms/SeparateConstOffsetFromGEP/AMDGPU/xor-to-or-disjoint.ll
@@ -4,8 +4,8 @@
; Test a simple case of xor to or disjoint transformation
-define void @test_basic_transformation(ptr %ptr, i64 %input) {
-; CHECK-LABEL: define void @test_basic_transformation(
+define half @test_basic_transformation(ptr %ptr, i64 %input) {
+; CHECK-LABEL: define half @test_basic_transformation(
; CHECK-SAME: ptr [[PTR:%.*]], i64 [[INPUT:%.*]]) {
; CHECK-NEXT: [[ENTRY:.*:]]
; CHECK-NEXT: [[BASE:%.*]] = and i64 [[INPUT]], -8192
@@ -18,7 +18,13 @@ define void @test_basic_transformation(ptr %ptr, i64 %input) {
; CHECK-NEXT: [[VAL1:%.*]] = load half, ptr [[GEP1]], align 2
; CHECK-NEXT: [[VAL2:%.*]] = load half, ptr [[GEP2]], align 2
; CHECK-NEXT: [[VAL3:%.*]] = load half, ptr [[GEP3]], align 2
-; CHECK-NEXT: ret void
+; CHECK-NEXT: [[VAL1_F:%.*]] = fpext half [[VAL1]] to float
+; CHECK-NEXT: [[VAL2_F:%.*]] = fpext half [[VAL2]] to float
+; CHECK-NEXT: [[VAL3_F:%.*]] = fpext half [[VAL3]] to float
+; CHECK-NEXT: [[SUM1_F:%.*]] = fadd float [[VAL1_F]], [[VAL2_F]]
+; CHECK-NEXT: [[SUM_TOTAL_F:%.*]] = fadd float [[SUM1_F]], [[VAL3_F]]
+; CHECK-NEXT: [[RESULT_H:%.*]] = fptrunc float [[SUM_TOTAL_F]] to half
+; CHECK-NEXT: ret half [[RESULT_H]]
;
entry:
%base = and i64 %input, -8192 ; Clear low bits
@@ -31,13 +37,19 @@ entry:
%val1 = load half, ptr %gep1
%val2 = load half, ptr %gep2
%val3 = load half, ptr %gep3
- ret void
+ %val1.f = fpext half %val1 to float
+ %val2.f = fpext half %val2 to float
+ %val3.f = fpext half %val3 to float
+ %sum1.f = fadd float %val1.f, %val2.f
+ %sum_total.f = fadd float %sum1.f, %val3.f
+ %result.h = fptrunc float %sum_total.f to half
+ ret half %result.h
}
; Test the decreasing order of offset xor to or disjoint transformation
-define void @test_descending_offset_transformation(ptr %ptr, i64 %input) {
-; CHECK-LABEL: define void @test_descending_offset_transformation(
+define half @test_descending_offset_transformation(ptr %ptr, i64 %input) {
+; CHECK-LABEL: define half @test_descending_offset_transformation(
; CHECK-SAME: ptr [[PTR:%.*]], i64 [[INPUT:%.*]]) {
; CHECK-NEXT: [[ENTRY:.*:]]
; CHECK-NEXT: [[BASE:%.*]] = and i64 [[INPUT]], -8192
@@ -51,7 +63,13 @@ define void @test_descending_offset_transformation(ptr %ptr, i64 %input) {
; CHECK-NEXT: [[VAL1:%.*]] = load half, ptr [[GEP1]], align 2
; CHECK-NEXT: [[VAL2:%.*]] = load half, ptr [[GEP2]], align 2
; CHECK-NEXT: [[VAL3:%.*]] = load half, ptr [[GEP3]], align 2
-; CHECK-NEXT: ret void
+; CHECK-NEXT: [[VAL1_F:%.*]] = fpext half [[VAL1]] to float
+; CHECK-NEXT: [[VAL2_F:%.*]] = fpext half [[VAL2]] to float
+; CHECK-NEXT: [[VAL3_F:%.*]] = fpext half [[VAL3]] to float
+; CHECK-NEXT: [[SUM1_F:%.*]] = fadd float [[VAL1_F]], [[VAL2_F]]
+; CHECK-NEXT: [[SUM_TOTAL_F:%.*]] = fadd float [[SUM1_F]], [[VAL3_F]]
+; CHECK-NEXT: [[RESULT_H:%.*]] = fptrunc float [[SUM_TOTAL_F]] to half
+; CHECK-NEXT: ret half [[RESULT_H]]
;
entry:
%base = and i64 %input, -8192 ; Clear low bits
@@ -64,13 +82,19 @@ entry:
%val1 = load half, ptr %gep1
%val2 = load half, ptr %gep2
%val3 = load half, ptr %gep3
- ret void
+ %val1.f = fpext half %val1 to float
+ %val2.f = fpext half %val2 to float
+ %val3.f = fpext half %val3 to float
+ %sum1.f = fadd float %val1.f, %val2.f
+ %sum_total.f = fadd float %sum1.f, %val3.f
+ %result.h = fptrunc float %sum_total.f to half
+ ret half %result.h
}
; Test that %addr2 is not transformed to or disjoint.
-define void @test_no_transfomation(ptr %ptr, i64 %input) {
-; CHECK-LABEL: define void @test_no_transfomation(
+define half @test_no_transfomation(ptr %ptr, i64 %input) {
+; CHECK-LABEL: define half @test_no_transfomation(
; CHECK-SAME: ptr [[PTR:%.*]], i64 [[INPUT:%.*]]) {
; CHECK-NEXT: [[ENTRY:.*:]]
; CHECK-NEXT: [[BASE:%.*]] = and i64 [[INPUT]], -8192
@@ -83,7 +107,13 @@ define void @test_no_transfomation(ptr %ptr, i64 %input) {
; CHECK-NEXT: [[VAL1:%.*]] = load half, ptr [[GEP1]], align 2
; CHECK-NEXT: [[VAL2:%.*]] = load half, ptr [[GEP2]], align 2
; CHECK-NEXT: [[VAL3:%.*]] = load half, ptr [[GEP3]], align 2
-; CHECK-NEXT: ret void
+; CHECK-NEXT: [[VAL1_F:%.*]] = fpext half [[VAL1]] to float
+; CHECK-NEXT: [[VAL2_F:%.*]] = fpext half [[VAL2]] to float
+; CHECK-NEXT: [[VAL3_F:%.*]] = fpext half [[VAL3]] to float
+; CHECK-NEXT: [[SUM1_F:%.*]] = fadd float [[VAL1_F]], [[VAL2_F]]
+; CHECK-NEXT: [[SUM_TOTAL_F:%.*]] = fadd float [[SUM1_F]], [[VAL3_F]]
+; CHECK-NEXT: [[RESULT_H:%.*]] = fptrunc float [[SUM_TOTAL_F]] to half
+; CHECK-NEXT: ret half [[RESULT_H]]
;
entry:
%base = and i64 %input, -8192 ; Clear low bits
@@ -96,13 +126,19 @@ entry:
%val1 = load half, ptr %gep1
%val2 = load half, ptr %gep2
%val3 = load half, ptr %gep3
- ret void
+ %val1.f = fpext half %val1 to float
+ %val2.f = fpext half %val2 to float
+ %val3.f = fpext half %val3 to float
+ %sum1.f = fadd float %val1.f, %val2.f
+ %sum_total.f = fadd float %sum1.f, %val3.f
+ %result.h = fptrunc float %sum_total.f to half
+ ret half %result.h
}
; Test case with xor instructions in different basic blocks
-define void @test_dom_tree(ptr %ptr, i64 %input, i1 %cond) {
-; CHECK-LABEL: define void @test_dom_tree(
+define half @test_dom_tree(ptr %ptr, i64 %input, i1 %cond) {
+; CHECK-LABEL: define half @test_dom_tree(
; CHECK-SAME: ptr [[PTR:%.*]], i64 [[INPUT:%.*]], i1 [[COND:%.*]]) {
; CHECK-NEXT: [[ENTRY:.*:]]
; CHECK-NEXT: [[BASE:%.*]] = and i64 [[INPUT]], -8192
@@ -121,10 +157,17 @@ define void @test_dom_tree(ptr %ptr, i64 %input, i1 %cond) {
; CHECK-NEXT: [[VAL3:%.*]] = load half, ptr [[GEP3]], align 2
; CHECK-NEXT: br label %[[MERGE]]
; CHECK: [[MERGE]]:
+; CHECK-NEXT: [[VAL_FROM_BRANCH:%.*]] = phi half [ [[VAL2]], %[[THEN]] ], [ [[VAL3]], %[[ELSE]] ]
; CHECK-NEXT: [[ADDR4_OR_DISJOINT:%.*]] = or disjoint i64 [[ADDR1]], 224
; CHECK-NEXT: [[GEP4:%.*]] = getelementptr i8, ptr [[PTR]], i64 [[ADDR4_OR_DISJOINT]]
; CHECK-NEXT: [[VAL4:%.*]] = load half, ptr [[GEP4]], align 2
-; CHECK-NEXT: ret void
+; CHECK-NEXT: [[VAL1_F:%.*]] = fpext half [[VAL1]] to float
+; CHECK-NEXT: [[VAL_FROM_BRANCH_F:%.*]] = fpext half [[VAL_FROM_BRANCH]] to float
+; CHECK-NEXT: [[VAL4_F:%.*]] = fpext half [[VAL4]] to float
+; CHECK-NEXT: [[SUM_INTERMEDIATE_F:%.*]] = fadd float [[VAL1_F]], [[VAL_FROM_BRANCH_F]]
+; CHECK-NEXT: [[FINAL_SUM_F:%.*]] = fadd float [[SUM_INTERMEDIATE_F]], [[VAL4_F]]
+; CHECK-NEXT: [[RESULT_H:%.*]] = fptrunc float [[FINAL_SUM_F]] to half
+; CHECK-NEXT: ret half [[RESULT_H]]
;
entry:
%base = and i64 %input, -8192 ; Clear low bits
@@ -146,9 +189,16 @@ else:
br label %merge
merge:
+ %val_from_branch = phi half [ %val2, %then ], [ %val3, %else ]
%addr4 = xor i64 %base, 240
%gep4 = getelementptr i8, ptr %ptr, i64 %addr4
%val4 = load half, ptr %gep4
- ret void
+ %val1.f = fpext half %val1 to float
+ %val_from_branch.f = fpext half %val_from_branch to float
+ %val4.f = fpext half %val4 to float
+ %sum_intermediate.f = fadd float %val1.f, %val_from_branch.f
+ %final_sum.f = fadd float %sum_intermediate.f, %val4.f
+ %result.h = fptrunc float %final_sum.f to half
+ ret half %result.h
}
More information about the llvm-commits
mailing list