[llvm] 85eeba4 - [ConstantHoisting] Add back ptr->ptr bitcast to avoid assertion failure

Yung, Douglas via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 14 03:47:47 PDT 2023


Hi Bjorn,

The test you added seems to be hitting an assertion failure on Windows build bots:
(https://lab.llvm.org/buildbot/#/builders/216/builds/25550)

Assertion failed: UsesNum == (ReBasesNum + NotRebasedNum) && "Not all uses are rebased", file Z:\b\llvm-clang-x86_64-sie-win\llvm-project\llvm\lib\Transforms\Scalar\ConstantHoisting.cpp, line 919

The failure is also appears in another build bot https://lab.llvm.org/buildbot/#/builders/233/builds/2011.

Is this assertion failure expected? Can you take a look?

Douglas Yung

-----Original Message-----
From: llvm-commits <llvm-commits-bounces at lists.llvm.org> On Behalf Of Bjorn Pettersson via llvm-commits
Sent: Monday, August 14, 2023 2:14
To: llvm-commits at lists.llvm.org
Subject: [llvm] 85eeba4 - [ConstantHoisting] Add back ptr->ptr bitcast to avoid assertion failure


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-overinde
+++ xing.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-reb
+++ ased.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 
+INVALID URI REMOVED
+/52689__;!!JmoZiZGBv3RvKRSx!5bL5AqmIJya5Tdog9D8MSk0ZnO2rhq0bIpQ7mYlCD6v
+Dt4xg3j8j-fhMZZCGsDd-zhsK-uG3b4Y_rb0Q812GnqeKXnm3$
+;
+; 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 
+ at g_77, i32 0, i32 3), %cleanup1660 ], [ null, %crit_edge ]
+  %l_323.sroa.2.0 = phi ptr [ getelementptr inbounds ([5 x i32], ptr 
+ at 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
+}


        
_______________________________________________
llvm-commits mailing list
llvm-commits at lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits 


More information about the llvm-commits mailing list