[PATCH] D138637: [InstCombine] Combine opaque pointer single index GEP and with src GEP which has result of array type

[Nikita Popov via Phabricator via llvm-commits]

nikic requested changes to this revision.
nikic added a comment.
This revision now requires changes to proceed.

I think in terms of general approach, the better way to handle this would be to relax the `Src->getResultElementType() != GEP.getSourceElementType()` check to allow the transform if the types can be made to match by adding additional zero indices.

(As a meta note, I think we'd probably be better off considering GEPs with multiple dynamic indices to be non-canonical, and try to split them up instead, e.g. because this allows LICM of an invariant part. But I guess that's not our current stance. It would be interesting to hear where you have found this merging to be beneficial.)



================
Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:2096
+    NewGEP->setIsInBounds(IsInBounds);
+    return NewGEP;
+  }
----------------
I believe this is missing a check that the GEP source element type matches the array element type of the Src result element type. Your current code would allow combining `[1 x float]` and `double` GEPs.


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

https://reviews.llvm.org/D138637