[PATCH] D22793: IR: Introduce inbounds attribute on getelementptr indices.

[Peter Collingbourne via llvm-commits]

pcc added inline comments.


================
Comment at: llvm/lib/Analysis/ConstantFolding.cpp:950
+  Constant *C = ConstantExpr::getGetElementPtr(
+      SrcElemTy, Ptr, NewIdxs, /*InBounds=*/false, InRangeIndex);
   assert(C->getType()->getPointerElementType() == Ty &&
----------------
efriedma wrote:
> Should we preserve inbounds here?  (Please add a FIXME if it's complicated somehow.)
My first impression is that it would be sound to preserve inbounds if all GEPs are inbounds, which is what D26441 does. Let's deal with that orthogonally though.


================
Comment at: llvm/lib/Analysis/ConstantFolding.cpp:970
 /// information, due to only being passed an opcode and operands. Constant
 /// folding using this function strips this information.
 ///
----------------
efriedma wrote:
> Does this patch affect this TODO somehow?
I guess it would be covered by the "etc" :)

But added to be explicit.


================
Comment at: llvm/lib/IR/ConstantFold.cpp:551
+               // information.
+               !cast<GEPOperator>(CE)->getInRangeIndex().hasValue()) {
       // If all of the indexes in the GEP are null values, there is no pointer
----------------
efriedma wrote:
> This is a little awkward... but I don't really see an alternative; it's okay, I guess.
Right, this should be no longer needed in the typeless pointer world anyway.


https://reviews.llvm.org/D22793