[PATCH] D28275: [EarlyCSE] infer conditional equalities within basic blocks

[Sanjoy Das via Phabricator via llvm-commits]

sanjoy requested changes to this revision.
sanjoy added a comment.
This revision now requires changes to proceed.

Comments inline.



================
Comment at: lib/Transforms/Scalar/EarlyCSE.cpp:620
+    // kicks in when we've inferred a control dependent equivelence fact.  Note
+    // that this does make the algorithm O(I^2) in the worst case (a series of
+    // calls which each use the previous instructions), but this was already
----------------
In the pathological case that you mentioned won't this be O(I^3), since you're doing:

```
for each Inst in BB:
  for each operand I of Inst:
    for each operand X of I:
      hash(X)
```

Maybe we can find an better bound as a function of the number of total //uses//?


================
Comment at: lib/Transforms/Scalar/EarlyCSE.cpp:674
+          DEBUG(dbgs() << "EarlyCSE CVP: Add conditional value for '"
+                << CondI->getName() << "' as true after "
+                << Inst->getName()  << "\n");
----------------
This bit is NFC right (just adds the debug output)?  If so, I'd just land this without review.


================
Comment at: test/Transforms/EarlyCSE/guards.ll:195
+  %cond0 = icmp slt i32 %val, 40
+  call void(i1,...) @llvm.experimental.guard(i1 %cond0) [ "deopt"() ]
+  call void(i1,...) @llvm.experimental.guard(i1 %cond0) [ "deopt"() ]
----------------
This will get the case where the second guard was using `%cond1 = <same expression as %cond0>` right?  If so, please add a test case that checks that.


https://reviews.llvm.org/D28275