[PATCH] [Polly] Added support for modulo accesses

Andreas Simbuerger simbuerg at fim.uni-passau.de
Wed Aug 13 14:49:55 PDT 2014


I have some stylistic comments inline, the rest looks good to me.

================
Comment at: lib/Analysis/ScopInfo.cpp:179
@@ +178,3 @@
+
+  // Pattern matching rules to capute some bit and modulo computations:
+  //
----------------
Johannes Doerfert wrote:
> Typo: capture
capture

================
Comment at: lib/Analysis/ScopInfo.cpp:188
@@ +187,3 @@
+  // Check for [A] and [C].
+  if (auto *OpAR = dyn_cast<SCEVAddRecExpr>(OpS)) {
+    assert(OpAR->getStepRecurrence(SE)->isOne());
----------------
Here 'auto' is ok, because I can infer the type from the context on the same line (imho).

================
Comment at: lib/Analysis/ScopInfo.cpp:191
@@ +190,3 @@
+
+    auto *OpARStart = OpAR->getStart();
+    auto *OpARStep = OpAR->getStepRecurrence(SE);
----------------
I would use the type here instead of 'auto'.

================
Comment at: lib/Analysis/ScopInfo.cpp:192
@@ +191,3 @@
+    auto *OpARStart = OpAR->getStart();
+    auto *OpARStep = OpAR->getStepRecurrence(SE);
+
----------------
I would use the type here instead of 'auto'.

================
Comment at: lib/Analysis/ScopInfo.cpp:311
@@ +310,3 @@
+  Value *Unknown = Expr->getValue();
+  if (BinaryOperator *BO = dyn_cast<BinaryOperator>(Unknown)) {
+    isl_pw_aff *RHS, *LHS;
----------------
Now you're not using 'auto', why?

================
Comment at: lib/Support/SCEVValidator.cpp:161
@@ -160,1 +160,3 @@
 
+    // Pattern matching rules to capute some bit and modulo computations:
+    //
----------------
Johannes Doerfert wrote:
> Typo: capture
capture

================
Comment at: lib/Support/SCEVValidator.cpp:172
@@ +171,3 @@
+    if (auto *OpAR = dyn_cast<SCEVAddRecExpr>(OpS)) {
+      auto *OpARStart = OpAR->getStart();
+
----------------
I would use the type here.

================
Comment at: lib/Support/SCEVValidator.cpp:379
@@ -348,2 +378,3 @@
 
-    if (Instruction *I = dyn_cast<Instruction>(Expr->getValue()))
+    if (Instruction *I = dyn_cast<Instruction>(Expr->getValue())) {
+      if (I->getOpcode() == Instruction::SRem) {
----------------
Consistency: auto?

http://reviews.llvm.org/D4843






More information about the llvm-commits mailing list