[PATCH] D20143: New pass: guard widening

[Andrew Trick via llvm-commits]

atrick added a comment.

I did a fairly quick review. Your pass looks pretty great.

I don't like to see unbounded recursion through multiple operands:

+  return all_of(Inst->operands(), [&](Value *Op) {
+      return isAvailableAt(Op, Loc);
+    });

...having had to fix many of these problems in LLVM when a
pathological case leads to exponential compile time. As a rule, I
think you either need a very small limit on the recursion, or a
visited set.

You're evaluating every guard against all dominating guard intrinsics:

+  for (unsigned i = 0, e = DFSI.getPathLength(); i != e; ++i) {
+    const auto &GuardsInCurBB = GuardsInBlock.find(CurBB)->second;
+    auto I = GuardsInCurBB.begin();
+    auto E = GuardsInCurBB.end();
+    for (auto *Candidate : make_range(I, E)) {

It's a nice simple design, but I wonder how well it scales (with
massive inlining). Since you're only matching guards with the same LHS
cmp operand, you could use a scoped hashtable instead. That way most
guards don't need to be searched. OTOH you would be maintaining a
complex side data structure.


http://reviews.llvm.org/D20143