[PATCH] D151082: [GuardUtils] Allow intermmediate blocks between widenable branch and deopt block

[Serguei Katkov via Phabricator via llvm-commits]

skatkov marked an inline comment as done.
skatkov added a comment.

In D151082#4363081 <https://reviews.llvm.org/D151082#4363081>, @apilipenko wrote:

> Why do we even care that an edge from a widenable condition branch terminates with a deopt? Widenable condition alone should be enough for widening.
>
> We might want to know that a widened condition will result in a deopt for profitability purposes. We know that a deopt path is very unlikely and the runtime will be able to heal from the widening if we go that path. But we can check for these concerns separately and explicitly. We can use BFI to determine if a path is rare. We can introduce a new metadata to mark branches that can be healed after widening.
>
> BTW, we use make implicit metadata for a similar purpose - to indicate that a branch can be "speculated" and the runtime can heal from this speculation.

Generally, I agree. But I propose to move step by step. This patch slightly increase the scope of optimization and does not touch what you mentioned as potential profitability.
I think that significant increase of an optimization scope might be done but it is a separate project. We can return to it back if needed.



================
Comment at: llvm/lib/Analysis/GuardUtils.cpp:42
+    }
+  } while (DeoptBB = DeoptBB->getUniqueSuccessor());
   return false;
----------------
nikic wrote:
> Could cause an infinite loop if DeoptBB is part of a trivial cycle?
Thanks, fixed.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151082/new/

https://reviews.llvm.org/D151082