[PATCH] D68244: [InstCombine] don't assume 'inbounds' for bitcast pointer to GEP transform (PR43501)

[Sanjay Patel via Phabricator via llvm-commits]

spatel marked 2 inline comments as done.
spatel added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp:2344-2345
+
+      // If the source pointer is known to be dereferenceable beyond the length
+      // of the destination type, then we can apply "inbounds" to the GEP.
+      bool CanBeNull;
----------------
lebedev.ri wrote:
> Why beyond? You check `>=`.
Yes, inaccurate comment. I'll fix it...


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp:2351
+          DerefBytes >= EltBitWidth / 8)
+        GEP->setIsInBounds();
+      return GEP;
----------------
lebedev.ri wrote:
> @jdoerfert @uenoku @sstefan1 
> attributor potential missing feature: i'd expect that it could set this `inbounds`, but it does not: https://godbolt.org/z/zkUt1-
Right...so I proposed this change based on the comments in PR43501, but it raises a question:
Do we want instcombine doing these limited improvements, or is it better to leave that entire responsibility to Attributor?


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

https://reviews.llvm.org/D68244