Skip to content

[InstCombine] Intersect nowrap flags between geps while folding into phi #115901

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Nov 13, 2024
Merged

[InstCombine] Intersect nowrap flags between geps while folding into phi #115901

merged 2 commits into from
Nov 13, 2024

Conversation

@antoniofrighetto
Copy link
Contributor

@antoniofrighetto antoniofrighetto commented Nov 12, 2024

A miscompilation issue has been addressed with refined checking.

Fixes: #115149.

@llvmbot llvmbot added debuginfo PGO Profile Guided Optimizations llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes llvm:transforms labels Nov 12, 2024
@llvmbot
Copy link
Member

llvmbot commented Nov 12, 2024 •
edited
Loading

@llvm/pr-subscribers-llvm-transforms
@llvm/pr-subscribers-pgo

@llvm/pr-subscribers-debuginfo

Author: Antonio Frighetto (antoniofrighetto)

Changes

A miscompilation issue has been addressed with refined checking.

Fixes: #115149.

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

8 Files Affected:

  • (modified) llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp (+5)
  • (modified) llvm/test/DebugInfo/Generic/instcombine-phi.ll (+1-3)
  • (modified) llvm/test/Transforms/InstCombine/gepphigep.ll (+6-5)
  • (modified) llvm/test/Transforms/InstCombine/new-inst-dbgloc-overwrite.ll (+4-3)
  • (modified) llvm/test/Transforms/InstCombine/opaque-ptr.ll (+9-3)
  • (modified) llvm/test/Transforms/InstCombine/phi-equal-incoming-pointers.ll (+45-18)
  • (modified) llvm/test/Transforms/InstCombine/phi.ll (+32-1)
  • (modified) llvm/test/Transforms/SampleProfile/pseudo-probe-instcombine.ll (+3-1)
diff --git a/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp b/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp
index cb5c4473051262..44154193038ac2 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp
@@ -554,6 +554,11 @@ Instruction *InstCombinerImpl::foldPHIArgGEPIntoPHI(PHINode &PN) {
         GEP->getSourceElementType() != FirstInst->getSourceElementType() ||
         GEP->getNumOperands() != FirstInst->getNumOperands())
       return nullptr;
+    // The resulting pointer from the phi-node may compute poison, if a
+    // getelementptr phi-operand carries inbounds semantics. Assume that the
+    // resulting phi-node is non-poison in order to perform the simplification.
+    if (!isGuaranteedNotToBeUndefOrPoison(V))
+      return nullptr;
 
     NW &= GEP->getNoWrapFlags();
 
diff --git a/llvm/test/DebugInfo/Generic/instcombine-phi.ll b/llvm/test/DebugInfo/Generic/instcombine-phi.ll
index 08f7bc07118084..ee4e6c96ec5236 100644
--- a/llvm/test/DebugInfo/Generic/instcombine-phi.ll
+++ b/llvm/test/DebugInfo/Generic/instcombine-phi.ll
@@ -105,8 +105,7 @@ if.end:                                           ; preds = %if.else, %if.then
 
 ; CHECK: define ptr @gep
 ; CHECK-LABEL: if.end:
-; CHECK: %[[PHI:.*]] = phi i64 [ %call, %if.then ], [ %call1, %if.else ]
-; CHECK: getelementptr inbounds i32, ptr %b, i64 %[[PHI]], !dbg [[gepMergedLoc:![0-9]+]]
+; CHECK: %[[PHI:.*]] = phi ptr [ %arrayidx, %if.then ], [ %arrayidx2, %if.else ]
 ; CHECK: ret ptr
 
 define ptr @gep(i32 %a, ptr %b) !dbg !23 {
@@ -300,7 +299,6 @@ declare ptr @bar3()
 
 ; CHECK: [[binopMergedLoc]] = !DILocation(line: 0
 ; CHECK: [[cmpMergedLoc]] = !DILocation(line: 0
-; CHECK: [[gepMergedLoc]] = !DILocation(line: 0
 ; CHECK: [[loadMergedLoc]] = !DILocation(line: 0
 ; CHECK: [[castMergedLoc]] = !DILocation(line: 0
 ; CHECK: [[binopConstMergedLoc]] = !DILocation(line: 0
diff --git a/llvm/test/Transforms/InstCombine/gepphigep.ll b/llvm/test/Transforms/InstCombine/gepphigep.ll
index 93266c67543963..7175111c4cdca1 100644
--- a/llvm/test/Transforms/InstCombine/gepphigep.ll
+++ b/llvm/test/Transforms/InstCombine/gepphigep.ll
@@ -86,12 +86,12 @@ define i32 @test3(ptr %dm, i1 %tmp4, i64 %tmp9, i64 %tmp19, i64 %tmp20, i64 %tmp
 ; CHECK:       bb3:
 ; CHECK-NEXT:    [[TMP0:%.*]] = phi i64 [ [[TMP19]], [[BB1]] ], [ [[TMP20]], [[BB2]] ]
 ; CHECK-NEXT:    [[TMP22:%.*]] = invoke i32 @foo1(i32 11)
-; CHECK-NEXT:    to label [[BB4:%.*]] unwind label [[BB5:%.*]]
+; CHECK-NEXT:            to label [[BB4:%.*]] unwind label [[BB5:%.*]]
 ; CHECK:       bb4:
 ; CHECK-NEXT:    ret i32 0
 ; CHECK:       bb5:
 ; CHECK-NEXT:    [[TMP27:%.*]] = landingpad { ptr, i32 }
-; CHECK-NEXT:    catch ptr @_ZTIi
+; CHECK-NEXT:            catch ptr @_ZTIi
 ; CHECK-NEXT:    [[TMP1:%.*]] = getelementptr inbounds [[STRUCT3]], ptr [[DM]], i64 [[TMP0]], i32 1
 ; CHECK-NEXT:    [[TMP35:%.*]] = getelementptr inbounds [[STRUCT4:%.*]], ptr [[TMP1]], i64 [[TMP21:%.*]], i32 1, i32 1
 ; CHECK-NEXT:    [[TMP25:%.*]] = load i32, ptr [[TMP35]], align 4
@@ -140,18 +140,19 @@ define ptr @test4(i32 %value, ptr %buffer) {
 ; CHECK-NEXT:    [[CMP:%.*]] = icmp ugt i32 [[VALUE:%.*]], 127
 ; CHECK-NEXT:    br i1 [[CMP]], label [[LOOP_HEADER:%.*]], label [[EXIT:%.*]]
 ; CHECK:       loop.header:
+; CHECK-NEXT:    [[BUFFER:%.*]] = getelementptr inbounds i8, ptr [[BUFFER1:%.*]], i64 1
 ; CHECK-NEXT:    br label [[LOOP_BODY:%.*]]
 ; CHECK:       loop.body:
-; CHECK-NEXT:    [[BUFFER_PN:%.*]] = phi ptr [ [[BUFFER:%.*]], [[LOOP_HEADER]] ], [ [[LOOPPTR:%.*]], [[LOOP_BODY]] ]
+; CHECK-NEXT:    [[BUFFER_PN:%.*]] = phi ptr [ [[BUFFER]], [[LOOP_HEADER]] ], [ [[LOOPPTR:%.*]], [[LOOP_BODY]] ]
 ; CHECK-NEXT:    [[NEWVAL:%.*]] = phi i32 [ [[VALUE]], [[LOOP_HEADER]] ], [ [[SHR:%.*]], [[LOOP_BODY]] ]
-; CHECK-NEXT:    [[LOOPPTR]] = getelementptr inbounds i8, ptr [[BUFFER_PN]], i64 1
 ; CHECK-NEXT:    [[SHR]] = lshr i32 [[NEWVAL]], 7
+; CHECK-NEXT:    [[LOOPPTR]] = getelementptr inbounds i8, ptr [[BUFFER_PN]], i64 1
 ; CHECK-NEXT:    [[CMP2:%.*]] = icmp ugt i32 [[NEWVAL]], 16383
 ; CHECK-NEXT:    br i1 [[CMP2]], label [[LOOP_BODY]], label [[LOOP_EXIT:%.*]]
 ; CHECK:       loop.exit:
 ; CHECK-NEXT:    br label [[EXIT]]
 ; CHECK:       exit:
-; CHECK-NEXT:    [[TMP0:%.*]] = phi ptr [ [[LOOPPTR]], [[LOOP_EXIT]] ], [ [[BUFFER]], [[ENTRY:%.*]] ]
+; CHECK-NEXT:    [[TMP0:%.*]] = phi ptr [ [[BUFFER_PN]], [[LOOP_EXIT]] ], [ [[BUFFER1]], [[ENTRY:%.*]] ]
 ; CHECK-NEXT:    [[INCPTR3:%.*]] = getelementptr inbounds i8, ptr [[TMP0]], i64 2
 ; CHECK-NEXT:    ret ptr [[INCPTR3]]
 ;
diff --git a/llvm/test/Transforms/InstCombine/new-inst-dbgloc-overwrite.ll b/llvm/test/Transforms/InstCombine/new-inst-dbgloc-overwrite.ll
index 2b933a2f9e0756..2084eb623a66f7 100644
--- a/llvm/test/Transforms/InstCombine/new-inst-dbgloc-overwrite.ll
+++ b/llvm/test/Transforms/InstCombine/new-inst-dbgloc-overwrite.ll
@@ -14,13 +14,14 @@ define void @test(ptr %xfA, ptr %xfB, i1 %cmp5) {
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    br i1 [[CMP5:%.*]], label [[IF_ELSE:%.*]], label [[IF_THEN6:%.*]]
 ; CHECK:       if.then6:
+; CHECK-NEXT:    [[XFB:%.*]] = getelementptr i8, ptr [[XFB1:%.*]], i64 4
 ; CHECK-NEXT:    br label [[IF_END11:%.*]]
 ; CHECK:       if.else:
+; CHECK-NEXT:    [[XFA:%.*]] = getelementptr i8, ptr [[XFA1:%.*]], i64 4
 ; CHECK-NEXT:    br label [[IF_END11]]
 ; CHECK:       if.end11:
-; CHECK-NEXT:    [[XFA_PN:%.*]] = phi ptr [ [[XFA:%.*]], [[IF_ELSE]] ], [ [[XFB:%.*]], [[IF_THEN6]] ]
-; CHECK-NEXT:    [[XF1_SROA_8_0_IN:%.*]] = getelementptr i8, ptr [[XFA_PN]], i64 4
-; CHECK-NEXT:    [[XF1_SROA_8_0:%.*]] = load float, ptr [[XF1_SROA_8_0_IN]], align 4, !dbg [[DBG3:![0-9]+]], !annotation [[META7:![0-9]+]]
+; CHECK-NEXT:    [[XFA_PN:%.*]] = phi ptr [ [[XFA]], [[IF_ELSE]] ], [ [[XFB]], [[IF_THEN6]] ]
+; CHECK-NEXT:    [[XF1_SROA_8_0:%.*]] = load float, ptr [[XFA_PN]], align 4, !dbg [[DBG3:![0-9]+]], !annotation [[META7:![0-9]+]]
 ; CHECK-NEXT:    [[CMP_I:%.*]] = fcmp ugt float [[XF1_SROA_8_0]], 0.000000e+00
 ; CHECK-NEXT:    br i1 [[CMP_I]], label [[IF_END_I:%.*]], label [[IF_THEN_I:%.*]]
 ; CHECK:       if.then.i:
diff --git a/llvm/test/Transforms/InstCombine/opaque-ptr.ll b/llvm/test/Transforms/InstCombine/opaque-ptr.ll
index a52a07389baee2..761fb6c89553d7 100644
--- a/llvm/test/Transforms/InstCombine/opaque-ptr.ll
+++ b/llvm/test/Transforms/InstCombine/opaque-ptr.ll
@@ -519,11 +519,13 @@ define ptr @phi_of_gep(i1 %c, ptr %p) {
 ; CHECK-LABEL: @phi_of_gep(
 ; CHECK-NEXT:    br i1 [[C:%.*]], label [[IF:%.*]], label [[ELSE:%.*]]
 ; CHECK:       if:
+; CHECK-NEXT:    [[GEP1:%.*]] = getelementptr i8, ptr [[P:%.*]], i64 4
 ; CHECK-NEXT:    br label [[JOIN:%.*]]
 ; CHECK:       else:
+; CHECK-NEXT:    [[GEP2:%.*]] = getelementptr i8, ptr [[P]], i64 4
 ; CHECK-NEXT:    br label [[JOIN]]
 ; CHECK:       join:
-; CHECK-NEXT:    [[PHI:%.*]] = getelementptr i8, ptr [[P:%.*]], i64 4
+; CHECK-NEXT:    [[PHI:%.*]] = phi ptr [ [[GEP1]], [[IF]] ], [ [[GEP2]], [[ELSE]] ]
 ; CHECK-NEXT:    ret ptr [[PHI]]
 ;
   br i1 %c, label %if, label %else
@@ -545,11 +547,13 @@ define ptr @phi_of_gep_flags_1(i1 %c, ptr %p) {
 ; CHECK-LABEL: @phi_of_gep_flags_1(
 ; CHECK-NEXT:    br i1 [[C:%.*]], label [[IF:%.*]], label [[ELSE:%.*]]
 ; CHECK:       if:
+; CHECK-NEXT:    [[GEP1:%.*]] = getelementptr inbounds i8, ptr [[P:%.*]], i64 4
 ; CHECK-NEXT:    br label [[JOIN:%.*]]
 ; CHECK:       else:
+; CHECK-NEXT:    [[GEP2:%.*]] = getelementptr nusw nuw i8, ptr [[P]], i64 4
 ; CHECK-NEXT:    br label [[JOIN]]
 ; CHECK:       join:
-; CHECK-NEXT:    [[PHI:%.*]] = getelementptr nusw nuw i8, ptr [[P:%.*]], i64 4
+; CHECK-NEXT:    [[PHI:%.*]] = phi ptr [ [[GEP1]], [[IF]] ], [ [[GEP2]], [[ELSE]] ]
 ; CHECK-NEXT:    ret ptr [[PHI]]
 ;
   br i1 %c, label %if, label %else
@@ -571,11 +575,13 @@ define ptr @phi_of_gep_flags_2(i1 %c, ptr %p) {
 ; CHECK-LABEL: @phi_of_gep_flags_2(
 ; CHECK-NEXT:    br i1 [[C:%.*]], label [[IF:%.*]], label [[ELSE:%.*]]
 ; CHECK:       if:
+; CHECK-NEXT:    [[GEP1:%.*]] = getelementptr nusw nuw i8, ptr [[P:%.*]], i64 4
 ; CHECK-NEXT:    br label [[JOIN:%.*]]
 ; CHECK:       else:
+; CHECK-NEXT:    [[GEP2:%.*]] = getelementptr nuw i8, ptr [[P]], i64 4
 ; CHECK-NEXT:    br label [[JOIN]]
 ; CHECK:       join:
-; CHECK-NEXT:    [[PHI:%.*]] = getelementptr nuw i8, ptr [[P:%.*]], i64 4
+; CHECK-NEXT:    [[PHI:%.*]] = phi ptr [ [[GEP1]], [[IF]] ], [ [[GEP2]], [[ELSE]] ]
 ; CHECK-NEXT:    ret ptr [[PHI]]
 ;
   br i1 %c, label %if, label %else
diff --git a/llvm/test/Transforms/InstCombine/phi-equal-incoming-pointers.ll b/llvm/test/Transforms/InstCombine/phi-equal-incoming-pointers.ll
index 998169ec2fe26f..4d8178b528c636 100644
--- a/llvm/test/Transforms/InstCombine/phi-equal-incoming-pointers.ll
+++ b/llvm/test/Transforms/InstCombine/phi-equal-incoming-pointers.ll
@@ -15,11 +15,13 @@ define i32 @test_gep_and_bitcast(i1 %cond, i1 %cond2) {
 ; ALL-NEXT:    [[OBJ:%.*]] = call ptr @get_ptr.i8()
 ; ALL-NEXT:    br i1 [[COND:%.*]], label [[BB1:%.*]], label [[BB2:%.*]]
 ; ALL:       bb1:
+; ALL-NEXT:    [[PTR1:%.*]] = getelementptr inbounds i8, ptr [[OBJ]], i64 16
 ; ALL-NEXT:    br label [[EXIT:%.*]]
 ; ALL:       bb2:
+; ALL-NEXT:    [[PTR2:%.*]] = getelementptr inbounds i8, ptr [[OBJ]], i64 16
 ; ALL-NEXT:    br label [[EXIT]]
 ; ALL:       exit:
-; ALL-NEXT:    [[PTR_TYPED:%.*]] = getelementptr inbounds i8, ptr [[OBJ]], i64 16
+; ALL-NEXT:    [[PTR_TYPED:%.*]] = phi ptr [ [[PTR1]], [[BB1]] ], [ [[PTR2]], [[BB2]] ]
 ; ALL-NEXT:    [[RES_PHI:%.*]] = load i32, ptr [[PTR_TYPED]], align 4
 ; ALL-NEXT:    store i32 1, ptr [[PTR_TYPED]], align 4
 ; ALL-NEXT:    [[RES:%.*]] = select i1 [[COND2:%.*]], i32 [[RES_PHI]], i32 1
@@ -53,11 +55,13 @@ define i32 @test_gep_and_bitcast_arg(ptr %obj, i1 %cond, i1 %cond2) {
 ; ALL-NEXT:  entry:
 ; ALL-NEXT:    br i1 [[COND:%.*]], label [[BB1:%.*]], label [[BB2:%.*]]
 ; ALL:       bb1:
+; ALL-NEXT:    [[PTR1:%.*]] = getelementptr inbounds i8, ptr [[OBJ:%.*]], i64 16
 ; ALL-NEXT:    br label [[EXIT:%.*]]
 ; ALL:       bb2:
+; ALL-NEXT:    [[PTR2:%.*]] = getelementptr inbounds i8, ptr [[OBJ]], i64 16
 ; ALL-NEXT:    br label [[EXIT]]
 ; ALL:       exit:
-; ALL-NEXT:    [[PTR_TYPED:%.*]] = getelementptr inbounds i8, ptr [[OBJ:%.*]], i64 16
+; ALL-NEXT:    [[PTR_TYPED:%.*]] = phi ptr [ [[PTR1]], [[BB1]] ], [ [[PTR2]], [[BB2]] ]
 ; ALL-NEXT:    [[RES_PHI:%.*]] = load i32, ptr [[PTR_TYPED]], align 4
 ; ALL-NEXT:    store i32 1, ptr [[PTR_TYPED]], align 4
 ; ALL-NEXT:    [[RES:%.*]] = select i1 [[COND2:%.*]], i32 [[RES_PHI]], i32 1
@@ -101,11 +105,13 @@ define i32 @test_gep_and_bitcast_phi(i1 %cond, i1 %cond2, i1 %cond3) {
 ; ALL-NEXT:    call void @foo.i8(ptr [[ANOTHER_PHI]])
 ; ALL-NEXT:    br i1 [[COND2:%.*]], label [[BB3:%.*]], label [[BB4:%.*]]
 ; ALL:       bb3:
+; ALL-NEXT:    [[PTR1:%.*]] = getelementptr inbounds i8, ptr [[OBJ]], i64 16
 ; ALL-NEXT:    br label [[EXIT:%.*]]
 ; ALL:       bb4:
+; ALL-NEXT:    [[PTR2:%.*]] = getelementptr inbounds i8, ptr [[OBJ]], i64 16
 ; ALL-NEXT:    br label [[EXIT]]
 ; ALL:       exit:
-; ALL-NEXT:    [[PTR_TYPED:%.*]] = getelementptr inbounds i8, ptr [[OBJ]], i64 16
+; ALL-NEXT:    [[PTR_TYPED:%.*]] = phi ptr [ [[PTR1]], [[BB3]] ], [ [[PTR2]], [[BB4]] ]
 ; ALL-NEXT:    [[RES_PHI:%.*]] = load i32, ptr [[PTR_TYPED]], align 4
 ; ALL-NEXT:    store i32 1, ptr [[PTR_TYPED]], align 4
 ; ALL-NEXT:    [[RES:%.*]] = select i1 [[COND3:%.*]], i32 [[RES_PHI]], i32 1
@@ -153,11 +159,13 @@ define i32 @test_gep_i32ptr(i1 %cond, i1 %cond2) {
 ; ALL-NEXT:    [[OBJ:%.*]] = call ptr @get_ptr.i32()
 ; ALL-NEXT:    br i1 [[COND:%.*]], label [[BB1:%.*]], label [[BB2:%.*]]
 ; ALL:       bb1:
+; ALL-NEXT:    [[PTR1_TYPED:%.*]] = getelementptr inbounds i8, ptr [[OBJ]], i64 64
 ; ALL-NEXT:    br label [[EXIT:%.*]]
 ; ALL:       bb2:
+; ALL-NEXT:    [[PTR2_TYPED:%.*]] = getelementptr inbounds i8, ptr [[OBJ]], i64 64
 ; ALL-NEXT:    br label [[EXIT]]
 ; ALL:       exit:
-; ALL-NEXT:    [[PTR_TYPED:%.*]] = getelementptr inbounds i8, ptr [[OBJ]], i64 64
+; ALL-NEXT:    [[PTR_TYPED:%.*]] = phi ptr [ [[PTR1_TYPED]], [[BB1]] ], [ [[PTR2_TYPED]], [[BB2]] ]
 ; ALL-NEXT:    [[RES_PHI:%.*]] = load i32, ptr [[PTR_TYPED]], align 4
 ; ALL-NEXT:    store i32 1, ptr [[PTR_TYPED]], align 4
 ; ALL-NEXT:    [[RES:%.*]] = select i1 [[COND2:%.*]], i32 [[RES_PHI]], i32 1
@@ -192,11 +200,13 @@ define i32 @test_gep_and_bitcast_gep_base_ptr(i1 %cond, i1 %cond2) {
 ; ALL-NEXT:    [[OBJ0:%.*]] = call ptr @get_ptr.i8()
 ; ALL-NEXT:    br i1 [[COND:%.*]], label [[BB1:%.*]], label [[BB2:%.*]]
 ; ALL:       bb1:
+; ALL-NEXT:    [[PTR1:%.*]] = getelementptr inbounds i8, ptr [[OBJ0]], i64 32
 ; ALL-NEXT:    br label [[EXIT:%.*]]
 ; ALL:       bb2:
+; ALL-NEXT:    [[PTR2:%.*]] = getelementptr inbounds i8, ptr [[OBJ0]], i64 32
 ; ALL-NEXT:    br label [[EXIT]]
 ; ALL:       exit:
-; ALL-NEXT:    [[PTR_TYPED:%.*]] = getelementptr inbounds i8, ptr [[OBJ0]], i64 32
+; ALL-NEXT:    [[PTR_TYPED:%.*]] = phi ptr [ [[PTR1]], [[BB1]] ], [ [[PTR2]], [[BB2]] ]
 ; ALL-NEXT:    [[RES_PHI:%.*]] = load i32, ptr [[PTR_TYPED]], align 4
 ; ALL-NEXT:    store i32 1, ptr [[PTR_TYPED]], align 4
 ; ALL-NEXT:    [[RES:%.*]] = select i1 [[COND2:%.*]], i32 [[RES_PHI]], i32 1
@@ -227,18 +237,33 @@ exit:
 }
 
 define i32 @test_gep_and_bitcast_same_bb(i1 %cond, i1 %cond2) {
-; ALL-LABEL: @test_gep_and_bitcast_same_bb(
-; ALL-NEXT:  entry:
-; ALL-NEXT:    [[OBJ:%.*]] = call ptr @get_ptr.i8()
-; ALL-NEXT:    br i1 [[COND:%.*]], label [[EXIT:%.*]], label [[BB2:%.*]]
-; ALL:       bb2:
-; ALL-NEXT:    br label [[EXIT]]
-; ALL:       exit:
-; ALL-NEXT:    [[PTR_TYPED:%.*]] = getelementptr inbounds i8, ptr [[OBJ]], i64 16
-; ALL-NEXT:    [[RES_PHI:%.*]] = load i32, ptr [[PTR_TYPED]], align 4
-; ALL-NEXT:    store i32 1, ptr [[PTR_TYPED]], align 4
-; ALL-NEXT:    [[RES:%.*]] = select i1 [[COND2:%.*]], i32 [[RES_PHI]], i32 1
-; ALL-NEXT:    ret i32 [[RES]]
+; INSTCOMBINE-LABEL: @test_gep_and_bitcast_same_bb(
+; INSTCOMBINE-NEXT:  entry:
+; INSTCOMBINE-NEXT:    [[OBJ:%.*]] = call ptr @get_ptr.i8()
+; INSTCOMBINE-NEXT:    [[PTR1:%.*]] = getelementptr inbounds i8, ptr [[OBJ]], i64 16
+; INSTCOMBINE-NEXT:    br i1 [[COND:%.*]], label [[EXIT:%.*]], label [[BB2:%.*]]
+; INSTCOMBINE:       bb2:
+; INSTCOMBINE-NEXT:    [[PTR2:%.*]] = getelementptr inbounds i8, ptr [[OBJ]], i64 16
+; INSTCOMBINE-NEXT:    br label [[EXIT]]
+; INSTCOMBINE:       exit:
+; INSTCOMBINE-NEXT:    [[PTR_TYPED:%.*]] = phi ptr [ [[PTR1]], [[ENTRY:%.*]] ], [ [[PTR2]], [[BB2]] ]
+; INSTCOMBINE-NEXT:    [[RES_PHI:%.*]] = load i32, ptr [[PTR_TYPED]], align 4
+; INSTCOMBINE-NEXT:    store i32 1, ptr [[PTR_TYPED]], align 4
+; INSTCOMBINE-NEXT:    [[RES:%.*]] = select i1 [[COND2:%.*]], i32 [[RES_PHI]], i32 1
+; INSTCOMBINE-NEXT:    ret i32 [[RES]]
+;
+; INSTCOMBINEGVN-LABEL: @test_gep_and_bitcast_same_bb(
+; INSTCOMBINEGVN-NEXT:  entry:
+; INSTCOMBINEGVN-NEXT:    [[OBJ:%.*]] = call ptr @get_ptr.i8()
+; INSTCOMBINEGVN-NEXT:    [[PTR1:%.*]] = getelementptr inbounds i8, ptr [[OBJ]], i64 16
+; INSTCOMBINEGVN-NEXT:    br i1 [[COND:%.*]], label [[EXIT:%.*]], label [[BB2:%.*]]
+; INSTCOMBINEGVN:       bb2:
+; INSTCOMBINEGVN-NEXT:    br label [[EXIT]]
+; INSTCOMBINEGVN:       exit:
+; INSTCOMBINEGVN-NEXT:    [[RES_PHI:%.*]] = load i32, ptr [[PTR1]], align 4
+; INSTCOMBINEGVN-NEXT:    store i32 1, ptr [[PTR1]], align 4
+; INSTCOMBINEGVN-NEXT:    [[RES:%.*]] = select i1 [[COND2:%.*]], i32 [[RES_PHI]], i32 1
+; INSTCOMBINEGVN-NEXT:    ret i32 [[RES]]
 ;
 entry:
   %obj = call ptr @get_ptr.i8()
@@ -318,11 +343,13 @@ define i8 @test_gep(i1 %cond, i1 %cond2) {
 ; ALL-NEXT:    [[OBJ:%.*]] = call ptr @get_ptr.i8()
 ; ALL-NEXT:    br i1 [[COND:%.*]], label [[BB1:%.*]], label [[BB2:%.*]]
 ; ALL:       bb1:
+; ALL-NEXT:    [[PTR1:%.*]] = getelementptr inbounds i8, ptr [[OBJ]], i64 16
 ; ALL-NEXT:    br label [[EXIT:%.*]]
 ; ALL:       bb2:
+; ALL-NEXT:    [[PTR2:%.*]] = getelementptr inbounds i8, ptr [[OBJ]], i64 16
 ; ALL-NEXT:    br label [[EXIT]]
 ; ALL:       exit:
-; ALL-NEXT:    [[PTR_TYPED:%.*]] = getelementptr inbounds i8, ptr [[OBJ]], i64 16
+; ALL-NEXT:    [[PTR_TYPED:%.*]] = phi ptr [ [[PTR1]], [[BB1]] ], [ [[PTR2]], [[BB2]] ]
 ; ALL-NEXT:    [[RES_PHI:%.*]] = load i8, ptr [[PTR_TYPED]], align 1
 ; ALL-NEXT:    store i8 1, ptr [[PTR_TYPED]], align 1
 ; ALL-NEXT:    [[RES:%.*]] = select i1 [[COND2:%.*]], i8 [[RES_PHI]], i8 1
diff --git a/llvm/test/Transforms/InstCombine/phi.ll b/llvm/test/Transforms/InstCombine/phi.ll
index 4e90a8f0ba4313..57a8afa76159ec 100644
--- a/llvm/test/Transforms/InstCombine/phi.ll
+++ b/llvm/test/Transforms/InstCombine/phi.ll
@@ -250,11 +250,13 @@ Exit:           ; preds = %Loop
 define ptr @test8(ptr %A, i1 %b) {
 ; CHECK-LABEL: @test8(
 ; CHECK-NEXT:  BB0:
+; CHECK-NEXT:    [[X:%.*]] = getelementptr inbounds i8, ptr [[A:%.*]], i64 4
 ; CHECK-NEXT:    br i1 [[B:%.*]], label [[BB1:%.*]], label [[BB2:%.*]]
 ; CHECK:       BB1:
+; CHECK-NEXT:    [[Y:%.*]] = getelementptr i8, ptr [[A]], i64 4
 ; CHECK-NEXT:    br label [[BB2]]
 ; CHECK:       BB2:
-; CHECK-NEXT:    [[C:%.*]] = getelementptr i8, ptr [[A:%.*]], i64 4
+; CHECK-NEXT:    [[C:%.*]] = phi ptr [ [[X]], [[BB0:%.*]] ], [ [[Y]], [[BB1]] ]
 ; CHECK-NEXT:    ret ptr [[C]]
 ;
 BB0:
@@ -2794,3 +2796,32 @@ BB4:                                             ; preds = %BB3, %BB2, %BB1, %BB
 BB5:                                             ; preds = %BB4
   ret void
 }
+
+define i64 @wrong_gep_arg_into_phi(ptr noundef %ptr) {
+; CHECK-LABEL: @wrong_gep_arg_into_phi(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[PTR:%.*]] = getelementptr i8, ptr [[PTR1:%.*]], i64 1
+; CHECK-NEXT:    br label [[FOR_COND:%.*]]
+; CHECK:       for.cond:
+; CHECK-NEXT:    [[PTR_PN:%.*]] = phi ptr [ [[PTR]], [[ENTRY:%.*]] ], [ [[DOTPN:%.*]], [[FOR_COND]] ]
+; CHECK-NEXT:    [[VAL:%.*]] = load i8, ptr [[PTR_PN]], align 1
+; CHECK-NEXT:    [[COND_NOT:%.*]] = icmp eq i8 [[VAL]], 0
+; CHECK-NEXT:    [[DOTPN]] = getelementptr inbounds nuw i8, ptr [[PTR_PN]], i64 1
+; CHECK-NEXT:    br i1 [[COND_NOT]], label [[EXIT:%.*]], label [[FOR_COND]]
+; CHECK:       exit:
+; CHECK-NEXT:    ret i64 0
+;
+entry:
+  %add.ptr = getelementptr i8, ptr %ptr, i64 1
+  br label %for.cond
+
+for.cond:                                         ; preds = %for.cond, %entry
+  %.pn = phi ptr [ %add.ptr, %entry ], [ %incdec.ptr, %for.cond ]
+  %val = load i8, ptr %.pn, align 1
+  %cond = icmp ne i8 %val, 0
+  %incdec.ptr = getelementptr inbounds nuw i8, ptr %.pn, i64 1
+  br i1 %cond, label %for.cond, label %exit
+
+exit:                                             ; preds = %for.cond
+  ret i64 0
+}
diff --git a/llvm/test/Transforms/SampleProfile/pseudo-probe-instcombine.ll b/llvm/test/Transforms/SampleProfile/pseudo-probe-instcombine.ll
index d1c15306a72d08..d6883d2d9c6319 100644
--- a/llvm/test/Transforms/SampleProfile/pseudo-probe-instcombine.ll
+++ b/llvm/test/Transforms/SampleProfile/pseudo-probe-instcombine.ll
@@ -28,16 +28,18 @@ define dso_local void @merge(ptr nocapture readonly %params) local_unnamed_addr
 ; CHECK-NEXT:    [[TOBOOL56_NOT:%.*]] = icmp eq i32 [[TMP1]], 0
 ; CHECK-NEXT:    br i1 [[TOBOOL56_NOT]], label [[IF_END109]], label [[IF_END109_THREAD:%.*]]
 ; CHECK:       if.end109.thread:
+; CHECK-NEXT:    [[MINPART4:%.*]] = getelementptr inbounds i8, ptr [[PARAMS]], i64 172
 ; CHECK-NEXT:    call void @llvm.pseudoprobe(i64 -6172701105289426098, i64 2, i32 0, i64 -1)
 ; CHECK-NEXT:    br label [[IF_THEN138:%.*]]
 ; CHECK:       if.end109:
+; CHECK-NEXT:    [[MINPART:%.*]] = getelementptr inbounds i8, ptr [[PARAMS]], i64 172
 ; CHECK-NEXT:    call void @llvm.pseudoprobe(i64 -6172701105289426098, i64 3, i32 0, i64 -1)
 ; CHECK-NEXT:    [[TOBOOL116_NOT:%.*]] = icmp eq i32 [[TMP1]], 0
 ; CHECK-NEXT:    br i1 [[TOBOOL116_NOT]], label [[IF_THEN117:%.*]], label [[IF_THEN138]]
 ; CHECK:       if.then117:
 ; CHECK-NEXT:    ret void
 ; CHECK:       if.then138:
-; CHECK-NEXT:    [[DOTIN:%.*]] = getelementptr inbounds i8, ptr [[PARAMS]], i64 172
+; CHECK-NEXT:    [[DOTIN:%.*]] = phi ptr [ [[MINPART4]], [[IF_END109_THREAD]] ], [ [[MINPART]], [[IF_END109]] ]
 ; CHECK-NEXT:    [[TMP2:%.*]] = load i32, ptr [[DOTIN]], align 4
 ; CHECK-NEXT:    [[TOBOOL139_NOT:%.*]] = icmp eq i32 [[TMP2]], 0
 ; CHECK-NEXT:    br i1 [[TOBOOL139_NOT]], label [[IF_ELSE147:%.*]], label [[IF_THEN140:%.*]]

nikic
nikic requested changes Nov 12, 2024
View reviewed changes
Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is the right fix. As far as I can tell this is just a bug in the flag intersection, because we end up ignoring the flags on the first instruction. Would doing this fix the issue?

GEPNoWrapFlags NW = FirstInst->getNoWrapFlags()

@antoniofrighetto antoniofrighetto force-pushed the feature/wrong-fold-gep-into-phi branch from 53b1105 to c914c25 Compare November 12, 2024 18:12
@antoniofrighetto
Copy link
Contributor Author

antoniofrighetto commented Nov 12, 2024

I don't think this is the right fix. As far as I can tell this is just a bug in the flag intersection, because we end up ignoring the flags on the first instruction. Would doing this fix the issue?

Updated, thank you! Admittedly, I thought about intersecting flags here, but felt like dropping flags (losing inbounds in the gep) at this point might have been unfavourable for later optimizations (so thought of preventing the folding altogether was "sort of" more robust).

@antoniofrighetto antoniofrighetto changed the title [InstCombine] Do not fold gep operand into phi unless not poison [InstCombine] Intersect nowrap flags between geps while folding into phi Nov 12, 2024
nikic
nikic approved these changes Nov 12, 2024
View reviewed changes
Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@antoniofrighetto antoniofrighetto force-pushed the feature/wrong-fold-gep-into-phi branch from c914c25 to e5e171a Compare November 13, 2024 18:40
@antoniofrighetto antoniofrighetto force-pushed the feature/wrong-fold-gep-into-phi branch from e5e171a to 929cbe7 Compare November 13, 2024 18:42
@antoniofrighetto antoniofrighetto merged commit 929cbe7 into llvm:main Nov 13, 2024
5 of 7 checks passed
@AZero13
Copy link
Contributor

AZero13 commented Nov 13, 2024

Do we want to backport this to 19.x since it mentions introduced in 18.1?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nikic nikic nikic approved these changes

@dtcxzyw dtcxzyw Awaiting requested review from dtcxzyw

@goldsteinn goldsteinn Awaiting requested review from goldsteinn

Assignees

No one assigned

Labels

debuginfo llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes llvm:transforms PGO Profile Guided Optimizations

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[clang] Miscompile with -O3 and -O0/1/2 since 18.1.0

4 participants

@antoniofrighetto @llvmbot @AZero13 @nikic