[PATCH] Assume GetElementPtr offsets to be inbounds

Tobias Grosser tobias at grosser.es
Mon Nov 24 23:53:55 PST 2014


Thanks for the review. I will commit it as soon as our LNT builders are green again.

================
Comment at: include/polly/ScopInfo.h:476
@@ +475,3 @@
+  /// time at least one iteration of the outer loop is executed. Hence, we
+  /// assume:
+  ///
----------------
jdoerfert wrote:
> What about n > 10 and m > 1? Shouldn't that be a violation too?
The first dimension does not carry any size information. I declared the array as A[][20] to make this clear.

================
Comment at: lib/Analysis/ScopInfo.cpp:850
@@ -849,1 +849,3 @@
 
+void ScopStmt::deriveAssumptionsFromGEP(GetElementPtrInst *Inst) {
+  int Dimension = 0;
----------------
jdoerfert wrote:
> I would rename Inst to GEP or GEPInst or sth. to make the type clear.
Done.

================
Comment at: lib/Analysis/ScopInfo.cpp:856
@@ +855,3 @@
+
+  if (auto *PtrTy = dyn_cast<PointerType>(Ty)) {
+    Dimension = 1;
----------------
jdoerfert wrote:
> Theoretically, we could make this a while and increment the dimension, however I don't think it will ever matter.
> However, stripping of structs might be worthwile?
Possibly. I do not have a test case yet. I think we can add it in a subsequent commit if found useful.

================
Comment at: lib/Analysis/ScopInfo.cpp:867
@@ +866,3 @@
+
+    const SCEV *Expr = Parent.getSE()->getSCEV(Inst->getOperand(1 + Dimension));
+
----------------
jdoerfert wrote:
> 1 + Dimension is Operand!
Fixed.

================
Comment at: lib/Analysis/ScopInfo.cpp:869
@@ +868,3 @@
+
+    if (isAffineExpr(&Parent.getRegion(), Expr, *Parent.getSE())) {
+      isl_pw_aff *AccessOffset = SCEVAffinator::getPwAff(this, Expr);
----------------
jdoerfert wrote:
> Please move Parent.getSE() before the loop
Fixed.

http://reviews.llvm.org/D6369






More information about the llvm-commits mailing list