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

[nikic]

Summary

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.

[lebedev.ri]

This LG, thank you.
I recommend first landing cleanup (isMergedGEPInBounds() thingy) and then the fix.

llvm/lib/Transforms/InstCombine/InstructionCombining.cpp

1930

This is the fix i presume?

1947

And this should be a follow-up NFC cleanup.

llvm/test/Transforms/InstCombine/getelementptr.ll

1204

I agree that this was a miscompilation.

[nikic]

I've moved the addition of the extra function into D72060, as it really only makes sense with the extra logic added there -- otherwise calling the function takes more characters than the logic it contains :)

llvm/lib/Transforms/InstCombine/InstructionCombining.cpp

if (auto *NewBinOp = dyn_cast<BinaryOperator>(NarrowBO)) {
     if (IsSext)
       NewBinOp->setHasNoSignedWrap();
     else
       NewBinOp->setHasNoUnsignedWrap();
   }
   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();
   Type *GEPEltType = GEP.getSourceElementType();
   if (Value *V = SimplifyGEPInst(GEPEltType, Ops, SQ.getWithInstruction(&GEP)))
     return replaceInstUsesWith(GEP, V);
 
   // For vector geps, use the generic demanded vector support.
if (EndsWithSequential) {
           SimplifyAddInst(GO1, SO1, false, false, SQ.getWithInstruction(&GEP));
       // Only do the combine when we are sure the cost after the
       // merge is never more than that before the merge.
       if (Sum == nullptr)
         return nullptr;
 
       // 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;
       }
       Indices.append(Src->op_begin()+1, Src->op_end()-1);
       Indices.push_back(Sum);
       Indices.append(GEP.op_begin()+2, GEP.op_end());
     } else if (isa<Constant>(*GEP.idx_begin()) &&
                cast<Constant>(*GEP.idx_begin())->isNullValue() &&
                Src->getNumOperands() != 1) {
       // Otherwise we can do the fold if the first index of the GEP is a zero
       Indices.append(Src->op_begin()+1, Src->op_end());
       Indices.append(GEP.idx_begin()+1, GEP.idx_end());
     }
 
     if (!Indices.empty())
-      return GEP.isInBounds() && Src->isInBounds()
+      return isMergedGEPInBounds(*Src, *cast<GEPOperator>(&GEP))
                  ? GetElementPtrInst::CreateInBounds(
                        Src->getSourceElementType(), Src->getOperand(0), Indices,
                        GEP.getName())
                  : GetElementPtrInst::Create(Src->getSourceElementType(),
                                              Src->getOperand(0), Indices,
                                              GEP.getName());
   }
 

llvm/test/Transforms/InstCombine/getelementptr.ll

 ;
   %base2 = getelementptr inbounds [1 x i32], [1 x i32]* %base, i64 %idx
   %ptr = getelementptr [1 x i32], [1 x i32]* %base2, i64 0, i64 0
   ret i32* %ptr
 }
 
 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
   %ptr2 = getelementptr inbounds i32, i32* %ptr1, i64 4
   ret i32* %ptr2
 }

llvm/test/Transforms/InstCombine/phi-equal-incoming-pointers.ll

 ; ALL-NEXT:    br i1 [[COND:%.*]], label [[BB1:%.*]], label [[BB2:%.*]]
 ; ALL:       bb1:
 ; ALL-NEXT:    [[PTR1:%.*]] = getelementptr inbounds i8, i8* [[OBJ]], i64 16
 ; ALL-NEXT:    [[PTR1_TYPED:%.*]] = bitcast i8* [[PTR1]] to i32*
 ; ALL-NEXT:    [[RES1:%.*]] = load i32, i32* [[PTR1_TYPED]], align 4
 ; 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]])
 ; ALL-NEXT:    br label [[EXIT]]
 ; ALL:       exit:
 ; ALL-NEXT:    [[PTR_TYPED:%.*]] = phi i32* [ [[PTR1_TYPED]], [[BB1]] ], [ [[PTR2_TYPED]], [[BB2]] ]
 ; ALL-NEXT:    [[RES_PHI:%.*]] = phi i32 [ [[RES1]], [[BB1]] ], [ [[RES2]], [[BB2]] ]
 ; ALL-NEXT:    store i32 1, i32* [[PTR_TYPED]], align 4

llvm/test/Transforms/InstCombine/pr26992.ll

 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt -instcombine -S < %s | FileCheck %s
 target triple = "x86_64-pc-windows-msvc"
 
 define i1 @test1(i8* %p) personality i32 (...)* @__CxxFrameHandler3 {
 ; CHECK-LABEL: @test1(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[A:%.*]] = getelementptr i8, i8* [[P:%.*]], i64 1
 ; 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:
 ; CHECK-NEXT:    [[C:%.*]] = phi i8* [ [[B]], [[INVOKE_CONT]] ], [ [[A]], [[ENTRY:%.*]] ]
 ; CHECK-NEXT:    [[TMP1:%.*]] = catchswitch within none [label %catch] unwind to caller
 ; CHECK:       catch:
 ; CHECK-NEXT:    [[TMP2:%.*]] = catchpad within [[TMP1]] [i8* null, i32 64, i8* null]
 ; CHECK-NEXT:    catchret from [[TMP2]] to label [[EXIT]]