[PATCH] Add more opportunities for constant folding in ScalarEvolution.

David Majnemer david.majnemer at gmail.com
Tue Dec 2 15:46:45 PST 2014


================
Comment at: lib/Analysis/ScalarEvolution.cpp:2212
@@ +2211,3 @@
+/// any of the add or multiply expressions in this SCEV contain a constant.
+static bool ContainsConstantSomewhere(const SCEV* Expr) {
+  if (isa<SCEVConstant>(Expr))
----------------
Please give this function a name that starts with a lower case letter.  Also, pointers bind to the variable name.

================
Comment at: lib/Analysis/ScalarEvolution.cpp:2216
@@ +2215,3 @@
+
+  const SCEVNAryExpr * Target = dyn_cast<const SCEVNAryExpr>(Expr);
+  if (!Target)
----------------
It's more natural in LLVM to write:
  const auto *Target = dyn_cast<SCEVNAryExpr>(Expr);

================
Comment at: lib/Analysis/ScalarEvolution.cpp:2220-2221
@@ +2219,4 @@
+
+  for (auto Operand = Target->op_begin(),
+         E = Target->op_end(); Operand != E; ++Operand)
+    if ((isa<SCEVAddExpr>(*Operand)) || (isa<SCEVMulExpr>(*Operand))) {
----------------
This would probably read easier as:
  for (const SCEV *Operand : Target->operands())

================
Comment at: lib/Analysis/ScalarEvolution.cpp:2220-2227
@@ +2219,10 @@
+
+  for (auto Operand = Target->op_begin(),
+         E = Target->op_end(); Operand != E; ++Operand)
+    if ((isa<SCEVAddExpr>(*Operand)) || (isa<SCEVMulExpr>(*Operand))) {
+      if (ContainsConstantSomewhere(*Operand))
+        return true;
+    } else if (isa<SCEVConstant>(*Operand)) {
+      return true;
+    }
+
----------------
majnemer wrote:
> This would probably read easier as:
>   for (const SCEV *Operand : Target->operands())
I think this would look a little more natural:

  for (const SCEV *Operand : Target->operands()) {
    if (isa<SCEVConstant>(*Operand))
      return true;
    if (isa<SCEVAddExpr>(*Operand) || isa<SCEVMulExpr>(*Operand))
      if (containsConstantSomewhere(*Operand))
        return true;
  }

================
Comment at: lib/Analysis/ScalarEvolution.cpp:2222
@@ +2221,3 @@
+         E = Target->op_end(); Operand != E; ++Operand)
+    if ((isa<SCEVAddExpr>(*Operand)) || (isa<SCEVMulExpr>(*Operand))) {
+      if (ContainsConstantSomewhere(*Operand))
----------------
These extra parens look strange.

http://reviews.llvm.org/D6487






More information about the llvm-commits mailing list