[PATCH] D11051: Extend LICM to hoist loop invariant GEP out
hfinkel at anl.gov
hfinkel at anl.gov
Sat Jul 25 17:25:24 PDT 2015
hfinkel added a comment.
Some comments about the inbounds changes. They also need to be covered by the regression tests.
================
Comment at: lib/Transforms/Scalar/LICM.cpp:1145
@@ +1144,3 @@
+ FirstGEP->setIsInBounds(false);
+ else if (Offset.ugt(ObjectSize))
+ FirstGEP->setIsInBounds(true);
----------------
I think this can be uge, because inbounds pointers are allowed to point to one byte past the end of the object (and thus, equal the size).
Actually, you want Offset.ule (the offset must be less than or equal to the size for it to be inbounds).
================
Comment at: lib/Transforms/Scalar/LICM.cpp:1147
@@ +1146,2 @@
+ FirstGEP->setIsInBounds(true);
+}
----------------
But you also need to set inbounds to false is the offset is larger than the size (even if the original GEP was inbounds). You need to set the inbounds on the second GEP to false.
Repository:
rL LLVM
http://reviews.llvm.org/D11051
More information about the llvm-commits
mailing list