[llvm] 85eeba4 - [ConstantHoisting] Add back ptr->ptr bitcast to avoid assertion failure
Bjorn Pettersson via llvm-commits
llvm-commits at lists.llvm.org
Mon Aug 14 02:13:35 PDT 2023
Author: Bjorn Pettersson
Date: 2023-08-14T11:13:13+02:00
New Revision: 85eeba48a0fefcdf6d12cf422d2060592f689823
URL: https://github.com/llvm/llvm-project/commit/85eeba48a0fefcdf6d12cf422d2060592f689823
DIFF: https://github.com/llvm/llvm-project/commit/85eeba48a0fefcdf6d12cf422d2060592f689823.diff
LOG: [ConstantHoisting] Add back ptr->ptr bitcast to avoid assertion failure
In commit a7ee80fab213fe7a a ptr->ptr bitcast was removed. But that
seem to cause "Expected an cast instruction!" assertions later in
that pass. This patch will add back the bitcast again.
This was a bit unexpected since there is no bitcast added after
creating the Add instruction in the else clause, but I guess there
is something special with the GetElementPtr scenario which makes this
bitcast needed to avoid such asserts.
This patch is also adding a reproducer for
https://github.com/llvm/llvm-project/issues/52689
that started to fail due to hitting the above mentioned assert. Now
it should end up hitting the assertion failure from #52689 again.
Added:
llvm/test/Transforms/ConstantHoisting/X86/pr52689-not-all-uses-rebased.ll
Modified:
llvm/lib/Transforms/Scalar/ConstantHoisting.cpp
llvm/test/Transforms/ConstantHoisting/AArch64/const-hoist-gep.ll
llvm/test/Transforms/ConstantHoisting/ARM/const-hoist-gep-overindexing.ll
llvm/test/Transforms/ConstantHoisting/ARM/const-hoist-gep.ll
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/Scalar/ConstantHoisting.cpp b/llvm/lib/Transforms/Scalar/ConstantHoisting.cpp
index 0c9dea343a25e7..3e5d979f11cc50 100644
--- a/llvm/lib/Transforms/Scalar/ConstantHoisting.cpp
+++ b/llvm/lib/Transforms/Scalar/ConstantHoisting.cpp
@@ -763,6 +763,8 @@ void ConstantHoistingPass::emitBaseConstants(Instruction *Base,
// Constant being rebased is a ConstantExpr.
Mat = GetElementPtrInst::Create(Type::getInt8Ty(*Ctx), Base, Adj->Offset,
"mat_gep", Adj->MatInsertPt);
+ // Hide it behind a bitcast.
+ Mat = new BitCastInst(Mat, Adj->Ty, "mat_bitcast", Adj->MatInsertPt);
} else
// Constant being rebased is a ConstantInt.
Mat = BinaryOperator::Create(Instruction::Add, Base, Adj->Offset,
diff --git a/llvm/test/Transforms/ConstantHoisting/AArch64/const-hoist-gep.ll b/llvm/test/Transforms/ConstantHoisting/AArch64/const-hoist-gep.ll
index e540b146f560f4..da0b2e68313cd5 100644
--- a/llvm/test/Transforms/ConstantHoisting/AArch64/const-hoist-gep.ll
+++ b/llvm/test/Transforms/ConstantHoisting/AArch64/const-hoist-gep.ll
@@ -22,11 +22,14 @@ define dso_local void @zot() {
; CHECK-NEXT: [[CONST:%.*]] = bitcast ptr getelementptr inbounds ([[TMP0:%.*]], ptr @global, i32 0, i32 4, i32 0, i32 0) to ptr
; CHECK-NEXT: store i32 undef, ptr [[CONST]], align 4
; CHECK-NEXT: [[MAT_GEP:%.*]] = getelementptr i8, ptr [[CONST]], i32 4
-; CHECK-NEXT: store i32 undef, ptr [[MAT_GEP]], align 4
+; CHECK-NEXT: [[MAT_BITCAST:%.*]] = bitcast ptr [[MAT_GEP]] to ptr
+; CHECK-NEXT: store i32 undef, ptr [[MAT_BITCAST]], align 4
; CHECK-NEXT: [[MAT_GEP1:%.*]] = getelementptr i8, ptr [[CONST]], i32 160
-; CHECK-NEXT: store i32 undef, ptr [[MAT_GEP1]], align 4
-; CHECK-NEXT: [[MAT_GEP2:%.*]] = getelementptr i8, ptr [[CONST]], i32 164
-; CHECK-NEXT: store i32 undef, ptr [[MAT_GEP2]], align 4
+; CHECK-NEXT: [[MAT_BITCAST2:%.*]] = bitcast ptr [[MAT_GEP1]] to ptr
+; CHECK-NEXT: store i32 undef, ptr [[MAT_BITCAST2]], align 4
+; CHECK-NEXT: [[MAT_GEP3:%.*]] = getelementptr i8, ptr [[CONST]], i32 164
+; CHECK-NEXT: [[MAT_BITCAST4:%.*]] = bitcast ptr [[MAT_GEP3]] to ptr
+; CHECK-NEXT: store i32 undef, ptr [[MAT_BITCAST4]], align 4
; CHECK-NEXT: ret void
;
bb:
diff --git a/llvm/test/Transforms/ConstantHoisting/ARM/const-hoist-gep-overindexing.ll b/llvm/test/Transforms/ConstantHoisting/ARM/const-hoist-gep-overindexing.ll
index 36ef7045fd05ee..08ce51cefadcfe 100644
--- a/llvm/test/Transforms/ConstantHoisting/ARM/const-hoist-gep-overindexing.ll
+++ b/llvm/test/Transforms/ConstantHoisting/ARM/const-hoist-gep-overindexing.ll
@@ -14,9 +14,11 @@ define void @test_inbounds() {
; CHECK-NEXT: [[CONST:%.*]] = bitcast ptr getelementptr inbounds ([[TMP0:%.*]], ptr @global, i32 0, i32 1, i32 0) to ptr
; CHECK-NEXT: store i16 undef, ptr [[CONST]], align 2
; CHECK-NEXT: [[MAT_GEP:%.*]] = getelementptr i8, ptr [[CONST]], i32 2
-; CHECK-NEXT: store i16 undef, ptr [[MAT_GEP]], align 2
+; CHECK-NEXT: [[MAT_BITCAST:%.*]] = bitcast ptr [[MAT_GEP]] to ptr
+; CHECK-NEXT: store i16 undef, ptr [[MAT_BITCAST]], align 2
; CHECK-NEXT: [[MAT_GEP1:%.*]] = getelementptr i8, ptr [[CONST]], i32 20
-; CHECK-NEXT: store i16 undef, ptr [[MAT_GEP1]], align 2
+; CHECK-NEXT: [[MAT_BITCAST2:%.*]] = bitcast ptr [[MAT_GEP1]] to ptr
+; CHECK-NEXT: store i16 undef, ptr [[MAT_BITCAST2]], align 2
; CHECK-NEXT: ret void
;
bb:
diff --git a/llvm/test/Transforms/ConstantHoisting/ARM/const-hoist-gep.ll b/llvm/test/Transforms/ConstantHoisting/ARM/const-hoist-gep.ll
index b5c58aaa367098..2a11a7a15b858a 100644
--- a/llvm/test/Transforms/ConstantHoisting/ARM/const-hoist-gep.ll
+++ b/llvm/test/Transforms/ConstantHoisting/ARM/const-hoist-gep.ll
@@ -24,10 +24,12 @@ define dso_local void @zot() {
; CHECK-NEXT: [[CONST:%.*]] = bitcast ptr getelementptr inbounds ([[TMP0]], ptr @global, i32 0, i32 4, i32 0, i32 0) to ptr
; CHECK-NEXT: store i32 undef, ptr [[CONST]], align 4
; CHECK-NEXT: [[MAT_GEP:%.*]] = getelementptr i8, ptr [[CONST]], i32 4
-; CHECK-NEXT: store i32 undef, ptr [[MAT_GEP]], align 4
+; CHECK-NEXT: [[MAT_BITCAST:%.*]] = bitcast ptr [[MAT_GEP]] to ptr
+; CHECK-NEXT: store i32 undef, ptr [[MAT_BITCAST]], align 4
; CHECK-NEXT: store i32 undef, ptr [[CONST1]], align 4
; CHECK-NEXT: [[MAT_GEP2:%.*]] = getelementptr i8, ptr [[CONST1]], i32 4
-; CHECK-NEXT: store i32 undef, ptr [[MAT_GEP2]], align 4
+; CHECK-NEXT: [[MAT_BITCAST3:%.*]] = bitcast ptr [[MAT_GEP2]] to ptr
+; CHECK-NEXT: store i32 undef, ptr [[MAT_BITCAST3]], align 4
; CHECK-NEXT: ret void
;
bb:
diff --git a/llvm/test/Transforms/ConstantHoisting/X86/pr52689-not-all-uses-rebased.ll b/llvm/test/Transforms/ConstantHoisting/X86/pr52689-not-all-uses-rebased.ll
new file mode 100644
index 00000000000000..00d77f9963e983
--- /dev/null
+++ b/llvm/test/Transforms/ConstantHoisting/X86/pr52689-not-all-uses-rebased.ll
@@ -0,0 +1,39 @@
+; RUN: not --crash opt -passes='consthoist' -S -o - -consthoist-gep=1 -mtriple=x86_64-unknown-linux-gnu < %s 2>&1 | FileCheck %s
+
+; REQUIRES: asserts
+
+; This is a reproducer for https://github.com/llvm/llvm-project/issues/52689
+;
+; opt: ../lib/Transforms/Scalar/ConstantHoisting.cpp:919: bool llvm::ConstantHoistingPass::emitBaseConstants(llvm::GlobalVariable *): Assertion `UsesNum == (ReBasesNum + NotRebasedNum) && "Not all uses are rebased"' failed.
+
+; CHECK: Assertion `UsesNum == (ReBasesNum + NotRebasedNum) && "Not all uses are rebased"' failed.
+
+
+ at g_77 = external global [5 x i32]
+
+define internal ptr @func_29(i1 %p1, i1 %p2, ptr %p3) {
+entry:
+ br i1 %p1, label %crit_edge, label %if.else3089
+
+crit_edge: ; preds = %entry
+ br label %for.cond1063
+
+for.cond1063: ; preds = %cleanup1660, %crit_edge
+ %l_323.sroa.0.0 = phi ptr [ getelementptr inbounds ([5 x i32], ptr @g_77, i32 0, i32 3), %cleanup1660 ], [ null, %crit_edge ]
+ %l_323.sroa.2.0 = phi ptr [ getelementptr inbounds ([5 x i32], ptr @g_77, i32 0, i32 3), %cleanup1660 ], [ null, %crit_edge ]
+ br i1 %p2, label %cleanup1660.thread, label %cleanup1674
+
+cleanup1660.thread: ; preds = %for.cond1063
+ br label %cleanup1674
+
+cleanup1660: ; No predecessors!
+ br label %for.cond1063
+
+cleanup1674: ; preds = %cleanup1660.thread, %for.cond1063
+ store ptr getelementptr inbounds ([5 x i32], ptr @g_77, i32 0, i32 1), ptr %p3, align 1
+ ret ptr null
+
+if.else3089: ; preds = %entry
+ store ptr getelementptr inbounds ([5 x i32], ptr @g_77, i32 0, i32 1), ptr %p3, align 1
+ ret ptr null
+}
More information about the llvm-commits
mailing list