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

[Nick Desaulniers via Phabricator via llvm-commits]

nickdesaulniers added a comment.

Ok, new idea for an approach based on feedback thus far (comments welcome and appreciated):

1. allow the use of `callbr` values along ANY edge; direct or indirect. i.e. restore https://reviews.llvm.org/D135997.
2. Create a new function pass (as part of `TargetPassConfig::addISelPasses` so it runs before isel on LLVM IR) that:
  1. for each basic block terminator if it's a callbr that has outputs that have uses:
    1. splits indirect edges that happen to also be critical edges (if any) (akin to https://reviews.llvm.org/D138078, but in a dedicated pass, not during ISEL but just before as part of the `TargetPassConfig::addISelPasses` pipeline)
    2. inserts a new SSA value produced from either a new instruction, intrinsic call, or perhaps even just a single entry phi. This will be used a marker during SelectionDAGBuilder to denote where phyreg to virtreg copies need to go.  It need not reference the corresponding `callbr` since we split critical edges in 2.A.a previously, so it's unambiguous that our corresponding `callbr` is the terminator of my lone predecessor.  (This will cause SelectionDAGBuilder's FunctionLoweringInfo to no longer retain the callbr value to virtreg mapping if there are no other users of the callbr value along the direct edge, but this is ok; see 3 below).
    3. Replaces the uses dominated by the indirect edge to use the newly created value.  We might need to insert phis where previously there were none (perhaps the SSA construction @efriedma refers to?).
3. Add a mapping from CallBrInst to either INLINEASM_BR MachineInstr or first defined virtreg (not sure yet), probably in SelectionDAGBuilder's FunctionLoweringInfo instance member. Create an entry in this mapping when lowering a callbr or inlineasm.  Consume this mapping info when visiting the "new value" created from 2.A.b above to insert COPYs as necessary.

By having a dedicated pass, we can better test these transforms in isolation rather than as part of SelectionDAGISEL or SelectionDAGBuilder, which can help us be more confident in the correctness.  This approach will create new SSA values along each indirect edge (as per @jyknight ), should not be too much overhead for programs that don't use `callbr` since we only need to check whether each basic block's terminator is a `callbr` (as per @void ). Then we do some SSA construction without new instructions or IR special cases (as per @efriedma ).  This should remove the special cases for `callbr` we have when computing dominance as well.

Having a dedicated pass will help us test oddities like a diamond control flow pattern, with the def via callbr in the top, then use in the bottom (with or without phi).  Without a phi will need one inserted with the new SSA value as one potential value.  With a phi we'll need to update the correct branch+value pair.

Thoughts?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139565