[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