[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