[PATCH] D139565: [IR] add new callbrpad instruction

[Nick Desaulniers via Phabricator via llvm-commits]

nickdesaulniers added a comment.

In D139565#3979289 <https://reviews.llvm.org/D139565#3979289>, @efriedma wrote:

> It's not clear to me an IR instruction is necessary.  I mean, I understand why you want a callbrpad-equivalent MachineInstr: the only existing infrastructure for jumping out of the middle of a block is the exception-handling infrastructure, so you want to handle callbr in a similar way.  I guess if you have a MachineInstr, but you don't have an IR instruction, doing the translation would involve SSA construction in isel, which is maybe a bit awkward.  But I'm not sure that's sufficient reason to introduce an IR instruction.  At the IR level, callbr is a terminator, and we produce the same value along all edges, so IR transforms don't benefit from distinguishing between direct and indirect edges.

Consider the following control flow pattern psuedocode, without `callbrpad`.

    %0 = callbr i32 ... to %direct [%indirect]
  direct:
    br label %direct_out
  direct_out:
    ret i32 %0
  indirect:
    br label %indirect_out
  indirect_out:
    ret i32 %0

Where would you insert the potentially-necessary COPYs?  Which MBBs would you mark as having physreg liveins?  SelectionDAG thinks the value of the callbr is the first virtreg COPY after the INLINEASM_BR, which is incorrect for output branches. SelectionDAG has a 1:1 mapping of Value* to virtual register (`FuncInfo.ValueMap` used in this patch).  @jyknight  suggested it might be nice to have distinct SSA values for each indirect edge.

> If we do decide to go with an IR instruction, making the callbrpad optional seems likely to cause confusion in IR passes, in terms of what transformations are actually legal.  To make this formulation work, indirect destinations have to be distinct from the direct destination.  (Otherwise, it's not clear whether values produced directly are actually available in a given block.)  If callbrpad is mandatory, this falls out in an obvious way: the direct destination can't contain a callbrpad, and the indirect destinations must contain one, so they can't get folded together.  If it's optional, we need extra checks in a bunch of places to make sure the destinations don't get folded together.

See `@test5` added in llvm/test/CodeGen/X86/callbr-asm-outputs-indirect-isel.ll, which is a case of common direct and indirect destination.  We wind up with superfluous copies in the parent MBB, but the correct registers from expanding the `callbrpad`.  It's not the most optimal code immediately after isel, but it looks correct to me, and those dead copies do get wiped out by later passes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139565