[PATCH] D33257: [JumpThreading] Replace uses of Condition safely

[Sanjoy Das via Phabricator via llvm-commits]

sanjoy added a comment.

Mostly minor stuff.



================
Comment at: lib/Transforms/Scalar/JumpThreading.cpp:263
+// guard/assume.
+static void RemoveCondIfSafe(Instruction *Cond, Value *ToVal) {
+  using namespace PatternMatch;
----------------
I'd call this `replaceFoldableUses`.  IMHO the key transform here is that you're replacing uses, the fact that you can also sometimes remove `Cond` is just a "fringe benefit".


================
Comment at: lib/Transforms/Scalar/JumpThreading.cpp:277
+  // correctly folded, based on the semantics of assume.
+  for (Instruction &I : make_range(
+           BB->getTerminator()->getIterator().getReverse(), BB->rend())) {
----------------
Perhaps this can be `Instruction &I : reverse(*BB)`?


================
Comment at: lib/Transforms/Scalar/JumpThreading.cpp:283
+      break;
+    if (!isGuaranteedToTransferExecutionToSuccessor(&I))
+      return;
----------------
I'd `break` out of the loop in both cases, and erase `Cond` only if it does not have uses.  That way we'll remove the condition in cases like:

```
cond = icmp;
exit(0);
use(cond);
assume(cond);
```



https://reviews.llvm.org/D33257