[PATCH] D81607: BreakCriticalEdges for callbr indirect dests

Bill Wendling via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jun 12 18:48:40 PDT 2020


void added a comment.

In D81607#2091010 <https://reviews.llvm.org/D81607#2091010>, @nickdesaulniers wrote:

> In D81607#2090768 <https://reviews.llvm.org/D81607#2090768>, @efriedma wrote:
>
> > > Disclaimer: I don't plan to rework callbr's operands before landing this. Was that a conditional LGTM?
> >
> > Wasn't intended to be conditional on any code changes.  Just that we don't plan to ever allow jumping to blockaddresses passed in as register operands.
> >
> > > I assume there's something important @fhahn was alluding to, but I'm not sure precisely what, and I would like to make sure everyone's happy with this.
> >
> > The reason we can't split edges coming out of indirectbr instructions is that in general, there is no way to fix the related blockaddresses in functions that contain multiple indirectbrs.  We can't rename the successor in one indirectbr without renaming it in all of them.  And if we rename it in all of them, the edge is still critical after the transform.
> >
> > We want to be sure the same issue doesn't apply to callbr instructions.  If it does, then we would need to fix the callers of SplitCriticalEdge, not SplitCriticalEdge itself.
>
>
> Wait, that sounds like a problem.  Imagine we have 3 `callbr`s, two that share the same indirect destination, and we decide to split one their critical edges. Then we update the `blockaddress` of that `callbr`, but not the others where the others' indirect branch now doesn't preserve the previous operations.  As if we added another `callbr` in the entry of this test case with an indirect destination of `%CallSiteBB`.


We can't really support PHI nodes in the indirect destinations of a callbr. It reverts to the original issue we had with the first proposal allowing callbr outputs to be live on the indirect branches. There's no good way to do this without messing the CFG up tremendously, and I suspect that it would be buggy as hell. So any use of a value in the path starting from an indirect block must dominate that indirect block or be defined within that path.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81607





More information about the llvm-commits mailing list