[PATCH] D13974: [IndVarSimplify] Rewrite loop exit values with their initial values from loop preheader
Sanjoy Das via llvm-commits
llvm-commits at lists.llvm.org
Tue Oct 27 00:48:14 PDT 2015
sanjoy accepted this revision.
sanjoy added a comment.
This revision is now accepted and ready to land.
lgtm with comments addressed
================
Comment at: lib/Transforms/Scalar/IndVarSimplify.cpp:743
@@ +742,3 @@
+ Cond = BI->getCondition();
+ } else if (auto *SI = dyn_cast<SwitchInst>(TermInst)) {
+ Cond = SI->getCondition();
----------------
Minor: remove the braces `{` form the `else if`.
================
Comment at: lib/Transforms/Scalar/IndVarSimplify.cpp:747
@@ +746,3 @@
+
+ // All non-instructions are loop-invariant.
+ if (auto *CondInst = dyn_cast<Instruction>(Cond))
----------------
]I'd change this to `isLoopInvariant` for now -- `make LoopInvariant` may hoist arithmetic etc. over control flow and this could be a regression e.g. in cases like
```
for ( ... )
if (rare_condition) {
int x = loop_invariant * 3;
if (x == 42) then exit;
}
```
hoisting `loop_invariant * 3` to the loop preheader will be a net regression.
Sorry for not bringing this up earlier.
Also, note that both `isLoopInvariant` and `makeLoopInvariant` take a `Value` so you don't need to manually cast to an `Instruction`.
http://reviews.llvm.org/D13974
More information about the llvm-commits
mailing list