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

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri accepted this revision.
lebedev.ri added a comment.
This revision is now accepted and ready to land.

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



================
Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:1930
       if (Src->getNumOperands() == 2) {
+        GEP.setIsInBounds(isMergedGEPInBounds(*Src, *cast<GEPOperator>(&GEP)));
         GEP.setOperand(0, Src->getOperand(0));
----------------
This is the fix i presume?


================
Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:1947
     if (!Indices.empty())
-      return GEP.isInBounds() && Src->isInBounds()
+      return isMergedGEPInBounds(*Src, *cast<GEPOperator>(&GEP))
                  ? GetElementPtrInst::CreateInBounds(
----------------
And this should be a follow-up NFC cleanup.


================
Comment at: llvm/test/Transforms/InstCombine/getelementptr.ll:1204
 ; 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]]
----------------
I agree that this was a miscompilation.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72059/new/

https://reviews.llvm.org/D72059