[llvm] AlignmentFromAssumptions should only track pointer operand users (PR #73370)
via llvm-commits
llvm-commits at lists.llvm.org
Wed Nov 29 12:43:18 PST 2023
https://github.com/alex-t updated https://github.com/llvm/llvm-project/pull/73370
>From 09bceafc5dbff302157c1affe945cf9c3325fee2 Mon Sep 17 00:00:00 2001
From: Alexander Timofeev <alexander.timofeev at amd.com>
Date: Fri, 24 Nov 2023 21:11:35 +0100
Subject: [PATCH 1/3] AlignmentFromAssumptions should not track the load result
users
---
.../Scalar/AlignmentFromAssumptions.cpp | 20 +++++++++----------
.../alignment-from-assumptions_dont_crash.ll | 16 +++++++++++++++
2 files changed, 26 insertions(+), 10 deletions(-)
create mode 100644 llvm/test/Transforms/AlignmentFromAssumptions/alignment-from-assumptions_dont_crash.ll
diff --git a/llvm/lib/Transforms/Scalar/AlignmentFromAssumptions.cpp b/llvm/lib/Transforms/Scalar/AlignmentFromAssumptions.cpp
index 63b7903ef955d9b..905ff2e80cd111a 100644
--- a/llvm/lib/Transforms/Scalar/AlignmentFromAssumptions.cpp
+++ b/llvm/lib/Transforms/Scalar/AlignmentFromAssumptions.cpp
@@ -83,11 +83,7 @@ static Align getNewAlignment(const SCEV *AASCEV, const SCEV *AlignSCEV,
const SCEV *OffSCEV, Value *Ptr,
ScalarEvolution *SE) {
const SCEV *PtrSCEV = SE->getSCEV(Ptr);
- // On a platform with 32-bit allocas, but 64-bit flat/global pointer sizes
- // (*cough* AMDGPU), the effective SCEV type of AASCEV and PtrSCEV
- // may disagree. Trunc/extend so they agree.
- PtrSCEV = SE->getTruncateOrZeroExtend(
- PtrSCEV, SE->getEffectiveSCEVType(AASCEV->getType()));
+
const SCEV *DiffSCEV = SE->getMinusSCEV(PtrSCEV, AASCEV);
if (isa<SCEVCouldNotCompute>(DiffSCEV))
return Align(1);
@@ -216,6 +212,7 @@ bool AlignmentFromAssumptionsPass::processAssumption(CallInst *ACall,
}
while (!WorkList.empty()) {
+ bool AddUsers = true;
Instruction *J = WorkList.pop_back_val();
if (LoadInst *LI = dyn_cast<LoadInst>(J)) {
if (!isValidAssumeForContext(ACall, J, DT))
@@ -226,6 +223,8 @@ bool AlignmentFromAssumptionsPass::processAssumption(CallInst *ACall,
LI->setAlignment(NewAlignment);
++NumLoadAlignChanged;
}
+ // The user of a Load uses data - not a pointer!
+ AddUsers = false;
} else if (StoreInst *SI = dyn_cast<StoreInst>(J)) {
if (!isValidAssumeForContext(ACall, J, DT))
continue;
@@ -267,11 +266,12 @@ bool AlignmentFromAssumptionsPass::processAssumption(CallInst *ACall,
// Now that we've updated that use of the pointer, look for other uses of
// the pointer to update.
Visited.insert(J);
- for (User *UJ : J->users()) {
- Instruction *K = cast<Instruction>(UJ);
- if (!Visited.count(K))
- WorkList.push_back(K);
- }
+ if (AddUsers)
+ for (User *UJ : J->users()) {
+ Instruction *K = cast<Instruction>(UJ);
+ if (!Visited.count(K))
+ WorkList.push_back(K);
+ }
}
return true;
diff --git a/llvm/test/Transforms/AlignmentFromAssumptions/alignment-from-assumptions_dont_crash.ll b/llvm/test/Transforms/AlignmentFromAssumptions/alignment-from-assumptions_dont_crash.ll
new file mode 100644
index 000000000000000..107d677cdbc271c
--- /dev/null
+++ b/llvm/test/Transforms/AlignmentFromAssumptions/alignment-from-assumptions_dont_crash.ll
@@ -0,0 +1,16 @@
+; Test that we don't crash.
+; RUN: opt < %s -passes=alignment-from-assumptions -S
+
+target datalayout = "e-p:64:64-p1:64:64-p2:32:32-p3:32:32-p4:64:64-p5:32:32-p6:32:32-p7:160:256:256:32-p8:128:128-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-S32-A5-G1-ni:7:8"
+target triple = "amdgcn-amd-amdhsa"
+
+define amdgpu_kernel void @vectorize_global_local(ptr addrspace(1) nocapture readonly %arg, ptr addrspace(3) nocapture %arg1) {
+bb:
+ %tmp2 = getelementptr inbounds i32, ptr addrspace(1) %arg, i64 1
+ call void @llvm.assume(i1 true) [ "align"(ptr addrspace(1) %tmp2, i64 4) ]
+ %tmp3 = load i32, ptr addrspace(1) %tmp2, align 4
+ %tmp4 = getelementptr inbounds i32, ptr addrspace(3) %arg1, i32 1
+ store i32 %tmp3, ptr addrspace(3) %tmp4, align 4
+ ret void
+}
+declare void @llvm.assume(i1 noundef)
>From 6e4189199b206e4423070830ba24811e7457d133 Mon Sep 17 00:00:00 2001
From: Alexander <alexander.timofeev at amd.com>
Date: Wed, 29 Nov 2023 21:36:24 +0100
Subject: [PATCH 2/3] AlignmentFromAssumptions should only track pointer
operand users
---
.../Scalar/AlignmentFromAssumptions.h | 4 +-
.../Scalar/AlignmentFromAssumptions.cpp | 25 ++++---
.../alignment-from-assumptions_dont_crash.ll | 68 ++++++++++++++++++-
3 files changed, 82 insertions(+), 15 deletions(-)
diff --git a/llvm/include/llvm/Transforms/Scalar/AlignmentFromAssumptions.h b/llvm/include/llvm/Transforms/Scalar/AlignmentFromAssumptions.h
index 10b6e1c6a21b639..83ee9c26fbd11c6 100644
--- a/llvm/include/llvm/Transforms/Scalar/AlignmentFromAssumptions.h
+++ b/llvm/include/llvm/Transforms/Scalar/AlignmentFromAssumptions.h
@@ -25,6 +25,7 @@ class AssumptionCache;
class DominatorTree;
class ScalarEvolution;
class SCEV;
+class AAResults;
struct AlignmentFromAssumptionsPass
: public PassInfoMixin<AlignmentFromAssumptionsPass> {
@@ -32,10 +33,11 @@ struct AlignmentFromAssumptionsPass
// Glue for old PM.
bool runImpl(Function &F, AssumptionCache &AC, ScalarEvolution *SE_,
- DominatorTree *DT_);
+ DominatorTree *DT_, AAResults *AA_);
ScalarEvolution *SE = nullptr;
DominatorTree *DT = nullptr;
+ AAResults *AA = nullptr;
bool extractAlignmentInfo(CallInst *I, unsigned Idx, Value *&AAPtr,
const SCEV *&AlignSCEV, const SCEV *&OffSCEV);
diff --git a/llvm/lib/Transforms/Scalar/AlignmentFromAssumptions.cpp b/llvm/lib/Transforms/Scalar/AlignmentFromAssumptions.cpp
index 905ff2e80cd111a..13e939521cbeb38 100644
--- a/llvm/lib/Transforms/Scalar/AlignmentFromAssumptions.cpp
+++ b/llvm/lib/Transforms/Scalar/AlignmentFromAssumptions.cpp
@@ -212,7 +212,6 @@ bool AlignmentFromAssumptionsPass::processAssumption(CallInst *ACall,
}
while (!WorkList.empty()) {
- bool AddUsers = true;
Instruction *J = WorkList.pop_back_val();
if (LoadInst *LI = dyn_cast<LoadInst>(J)) {
if (!isValidAssumeForContext(ACall, J, DT))
@@ -223,8 +222,6 @@ bool AlignmentFromAssumptionsPass::processAssumption(CallInst *ACall,
LI->setAlignment(NewAlignment);
++NumLoadAlignChanged;
}
- // The user of a Load uses data - not a pointer!
- AddUsers = false;
} else if (StoreInst *SI = dyn_cast<StoreInst>(J)) {
if (!isValidAssumeForContext(ACall, J, DT))
continue;
@@ -265,13 +262,17 @@ bool AlignmentFromAssumptionsPass::processAssumption(CallInst *ACall,
// Now that we've updated that use of the pointer, look for other uses of
// the pointer to update.
- Visited.insert(J);
- if (AddUsers)
- for (User *UJ : J->users()) {
- Instruction *K = cast<Instruction>(UJ);
- if (!Visited.count(K))
- WorkList.push_back(K);
+ if (auto UJ = dyn_cast<User>(J))
+ for (auto &U : UJ->uses()) {
+ if (U->getType()->isPointerTy()) {
+ if (AA->alias(U, AAPtr)) {
+ Instruction *K = cast<Instruction>(U.getUser());
+ if (!Visited.count(K))
+ WorkList.push_back(K);
+ }
+ }
}
+
}
return true;
@@ -279,9 +280,10 @@ bool AlignmentFromAssumptionsPass::processAssumption(CallInst *ACall,
bool AlignmentFromAssumptionsPass::runImpl(Function &F, AssumptionCache &AC,
ScalarEvolution *SE_,
- DominatorTree *DT_) {
+ DominatorTree *DT_, AAResults *AA_) {
SE = SE_;
DT = DT_;
+ AA = AA_;
bool Changed = false;
for (auto &AssumeVH : AC.assumptions())
@@ -300,7 +302,8 @@ AlignmentFromAssumptionsPass::run(Function &F, FunctionAnalysisManager &AM) {
AssumptionCache &AC = AM.getResult<AssumptionAnalysis>(F);
ScalarEvolution &SE = AM.getResult<ScalarEvolutionAnalysis>(F);
DominatorTree &DT = AM.getResult<DominatorTreeAnalysis>(F);
- if (!runImpl(F, AC, &SE, &DT))
+ AAResults &AA = AM.getResult<AAManager>(F);
+ if (!runImpl(F, AC, &SE, &DT, &AA))
return PreservedAnalyses::all();
PreservedAnalyses PA;
diff --git a/llvm/test/Transforms/AlignmentFromAssumptions/alignment-from-assumptions_dont_crash.ll b/llvm/test/Transforms/AlignmentFromAssumptions/alignment-from-assumptions_dont_crash.ll
index 107d677cdbc271c..e55200aad44ae91 100644
--- a/llvm/test/Transforms/AlignmentFromAssumptions/alignment-from-assumptions_dont_crash.ll
+++ b/llvm/test/Transforms/AlignmentFromAssumptions/alignment-from-assumptions_dont_crash.ll
@@ -1,16 +1,78 @@
; Test that we don't crash.
-; RUN: opt < %s -passes=alignment-from-assumptions -S
+; RUN: opt < %s -passes=alignment-from-assumptions -S | FileCheck %s
target datalayout = "e-p:64:64-p1:64:64-p2:32:32-p3:32:32-p4:64:64-p5:32:32-p6:32:32-p7:160:256:256:32-p8:128:128-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-S32-A5-G1-ni:7:8"
target triple = "amdgcn-amd-amdhsa"
-define amdgpu_kernel void @vectorize_global_local(ptr addrspace(1) nocapture readonly %arg, ptr addrspace(3) nocapture %arg1) {
+define amdgpu_kernel void @test_gep(ptr addrspace(1) nocapture readonly %arg, ptr addrspace(3) nocapture %arg1) {
+; CHECK-LABEL: @test_gep
+; GEPs are supported so the alignment is changed from 2 to 4
+; CHECK: load i32, ptr addrspace(1) %tmp2, align 4
bb:
%tmp2 = getelementptr inbounds i32, ptr addrspace(1) %arg, i64 1
call void @llvm.assume(i1 true) [ "align"(ptr addrspace(1) %tmp2, i64 4) ]
- %tmp3 = load i32, ptr addrspace(1) %tmp2, align 4
+ %tmp3 = load i32, ptr addrspace(1) %tmp2, align 2
%tmp4 = getelementptr inbounds i32, ptr addrspace(3) %arg1, i32 1
store i32 %tmp3, ptr addrspace(3) %tmp4, align 4
ret void
}
+
+define amdgpu_kernel void @test_phi(ptr addrspace(1) nocapture readonly %arg, i32 %idx, ptr addrspace(3) nocapture %arg1) {
+; CHECK-LABEL: @test_phi
+; PHI is not supported - align 2 not changed
+; CHECK: load i32, ptr addrspace(1) %tmp2, align 2
+bb:
+ %cond = icmp ugt i32 %idx, 10
+ br i1 %cond, label %bb1, label %bb2
+
+bb1:
+ %gep1 = getelementptr i32, ptr addrspace(1) %arg, i32 6
+ br label %bb3
+
+bb2:
+ %gep2 = getelementptr i32, ptr addrspace(1) %arg, i32 7
+ br label %bb3
+
+bb3:
+ %gep3 = phi ptr addrspace(1) [%gep1, %bb1], [%gep2, %bb2]
+ %tmp2 = getelementptr inbounds i32, ptr addrspace(1) %gep3, i64 4
+ call void @llvm.assume(i1 true) [ "align"(ptr addrspace(1) %arg, i64 4) ]
+ %tmp3 = load i32, ptr addrspace(1) %tmp2, align 2
+ %tmp4 = getelementptr inbounds i32, ptr addrspace(3) %arg1, i32 1
+ store i32 %tmp3, ptr addrspace(3) %tmp4, align 4
+ ret void
+}
+
+define amdgpu_kernel void @test_select(ptr addrspace(1) nocapture readonly %arg, i32 %idx, ptr addrspace(3) nocapture %arg1) {
+; CHECK-LABEL: @test_select
+; select is not supported - align 2 not changed
+; CHECK: load i32, ptr addrspace(1) %tmp2, align 2
+bb:
+ %cond = icmp ugt i32 %idx, 10
+ %off1_gep = getelementptr i32, ptr addrspace(1) %arg, i32 6
+ %off2_gep = getelementptr i32, ptr addrspace(1) %arg, i32 7
+ %tmp2 = select i1 %cond, ptr addrspace(1) %off1_gep, ptr addrspace(1) %off2_gep
+ call void @llvm.assume(i1 true) [ "align"(ptr addrspace(1) %arg, i64 4) ]
+ %tmp3 = load i32, ptr addrspace(1) %tmp2, align 2
+ %tmp4 = getelementptr inbounds i32, ptr addrspace(3) %arg1, i32 1
+ store i32 %tmp3, ptr addrspace(3) %tmp4, align 4
+ ret void
+}
+
+define amdgpu_kernel void @test_cast(ptr addrspace(1) nocapture readonly %arg, i32 %idx, ptr addrspace(3) nocapture %arg1) {
+bb:
+; CHECK-LABEL: @test_cast
+; addrspacecast is not supported - align 2 not changed
+; CHECK: load i32, ptr addrspace(1) %tmp2, align 2
+; store is a user of the GEP so, align 2 is changed to 4
+; CHECK: store i32 %tmp3, ptr addrspace(3) %tmp4, align 4
+ %cast = addrspacecast ptr addrspace(3) %arg1 to ptr addrspace(1)
+ %tmp2 = getelementptr i32, ptr addrspace(1) %cast
+ call void @llvm.assume(i1 true) [ "align"(ptr addrspace(3) %arg1, i64 4) ]
+ %tmp3 = load i32, ptr addrspace(1) %tmp2, align 2
+ %tmp4 = getelementptr inbounds i32, ptr addrspace(3) %arg1, i32 1
+ store i32 %tmp3, ptr addrspace(3) %tmp4, align 2
+ ret void
+}
+
declare void @llvm.assume(i1 noundef)
>From e7ac710739dcbe07485b14131d9ba84792c7c9b2 Mon Sep 17 00:00:00 2001
From: Alexander <alexander.timofeev at amd.com>
Date: Wed, 29 Nov 2023 21:42:06 +0100
Subject: [PATCH 3/3] Whitespace error corrected
---
llvm/lib/Transforms/Scalar/AlignmentFromAssumptions.cpp | 1 -
1 file changed, 1 deletion(-)
diff --git a/llvm/lib/Transforms/Scalar/AlignmentFromAssumptions.cpp b/llvm/lib/Transforms/Scalar/AlignmentFromAssumptions.cpp
index 13e939521cbeb38..27422e7d8bb3423 100644
--- a/llvm/lib/Transforms/Scalar/AlignmentFromAssumptions.cpp
+++ b/llvm/lib/Transforms/Scalar/AlignmentFromAssumptions.cpp
@@ -272,7 +272,6 @@ bool AlignmentFromAssumptionsPass::processAssumption(CallInst *ACall,
}
}
}
-
}
return true;
More information about the llvm-commits
mailing list