[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