[PATCH] D72059: [InstCombine] Fix incorrect inbounds on GEP of GEP (PR44425)

[Nikita Popov via Phabricator via llvm-commits]

nikic created this revision.
nikic added reviewers: lebedev.ri, spatel, RKSimon.
Herald added subscribers: llvm-commits, hiraditya.
Herald added a project: LLVM.

This fixes https://bugs.llvm.org/show_bug.cgi?id=44425. We need to drop inbounds if one of the GEPs is not inbounds. This was already done when creating a new GEP, but not when modifying in place.

I'll followup with another patch to special-case the zero-index case to address https://bugs.llvm.org/show_bug.cgi?id=44423.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72059

Files:
  llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
  llvm/test/Transforms/InstCombine/getelementptr.ll
  llvm/test/Transforms/InstCombine/phi-equal-incoming-pointers.ll
  llvm/test/Transforms/InstCombine/pr26992.ll


Index: llvm/test/Transforms/InstCombine/pr26992.ll
===================================================================
--- llvm/test/Transforms/InstCombine/pr26992.ll
+++ llvm/test/Transforms/InstCombine/pr26992.ll
@@ -9,7 +9,7 @@
 ; CHECK-NEXT:    invoke void @may_throw()
 ; CHECK-NEXT:    to label [[INVOKE_CONT:%.*]] unwind label [[CATCH_DISPATCH:%.*]]
 ; CHECK:       invoke.cont:
-; CHECK-NEXT:    [[B:%.*]] = getelementptr inbounds i8, i8* [[P]], i64 2
+; CHECK-NEXT:    [[B:%.*]] = getelementptr i8, i8* [[P]], i64 2
 ; CHECK-NEXT:    invoke void @may_throw()
 ; CHECK-NEXT:    to label [[EXIT:%.*]] unwind label [[CATCH_DISPATCH]]
 ; CHECK:       catch.dispatch:
Index: llvm/test/Transforms/InstCombine/phi-equal-incoming-pointers.ll
===================================================================
--- llvm/test/Transforms/InstCombine/phi-equal-incoming-pointers.ll
+++ llvm/test/Transforms/InstCombine/phi-equal-incoming-pointers.ll
@@ -528,7 +528,7 @@
 ; ALL-NEXT:    call void @foo.i32(i32* nonnull [[PTR1_TYPED]])
 ; ALL-NEXT:    br label [[EXIT:%.*]]
 ; ALL:       bb2:
-; ALL-NEXT:    [[PTR2_1:%.*]] = getelementptr inbounds i8, i8* [[OBJ]], i64 16
+; ALL-NEXT:    [[PTR2_1:%.*]] = getelementptr i8, i8* [[OBJ]], i64 16
 ; ALL-NEXT:    [[PTR2_TYPED:%.*]] = bitcast i8* [[PTR2_1]] to i32*
 ; ALL-NEXT:    [[RES2:%.*]] = load i32, i32* [[PTR2_TYPED]], align 4
 ; ALL-NEXT:    call void @foo.i32(i32* nonnull [[PTR2_TYPED]])
Index: llvm/test/Transforms/InstCombine/getelementptr.ll
===================================================================
--- llvm/test/Transforms/InstCombine/getelementptr.ll
+++ llvm/test/Transforms/InstCombine/getelementptr.ll
@@ -1201,7 +1201,7 @@
 
 define i32* @test_gep_inbounds_of_gep(i32* %base) {
 ; CHECK-LABEL: @test_gep_inbounds_of_gep(
-; CHECK-NEXT:    [[PTR2:%.*]] = getelementptr inbounds i32, i32* [[BASE:%.*]], i64 8
+; CHECK-NEXT:    [[PTR2:%.*]] = getelementptr i32, i32* [[BASE:%.*]], i64 8
 ; CHECK-NEXT:    ret i32* [[PTR2]]
 ;
   %ptr1 = getelementptr i32, i32* %base, i64 4
Index: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
===================================================================
--- llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -1649,6 +1649,11 @@
   return CastInst::Create(CastOpc, NarrowBO, BO.getType());
 }
 
+bool isMergedGEPInBounds(GEPOperator &GEP1, GEPOperator &GEP2) {
+  // TODO: Special case zero-index GEPs.
+  return GEP1.isInBounds() && GEP2.isInBounds();
+}
+
 Instruction *InstCombiner::visitGetElementPtrInst(GetElementPtrInst &GEP) {
   SmallVector<Value*, 8> Ops(GEP.op_begin(), GEP.op_end());
   Type *GEPType = GEP.getType();
@@ -1922,6 +1927,7 @@
 
       // Update the GEP in place if possible.
       if (Src->getNumOperands() == 2) {
+        GEP.setIsInBounds(isMergedGEPInBounds(*Src, *cast<GEPOperator>(&GEP)));
         GEP.setOperand(0, Src->getOperand(0));
         GEP.setOperand(1, Sum);
         return &GEP;
@@ -1938,7 +1944,7 @@
     }
 
     if (!Indices.empty())
-      return GEP.isInBounds() && Src->isInBounds()
+      return isMergedGEPInBounds(*Src, *cast<GEPOperator>(&GEP))
                  ? GetElementPtrInst::CreateInBounds(
                        Src->getSourceElementType(), Src->getOperand(0), Indices,
                        GEP.getName())


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D72059.235794.patch
Type: text/x-patch
Size: 3370 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200101/d7caa957/attachment.bin>