[PATCH] D76961: [BranchFolder] don't remove MBB's that have their address taken

Mon Mar 30 16:57:20 PDT 2020

nickdesaulniers added a comment.

In D76961#1951241 <https://reviews.llvm.org/D76961#1951241>, @efriedma wrote:

> > Probably in InstrEmitter::EmitSpecialNode.
>
> We normally construct the MachineFunction CFG much earlier; see SelectionDAGBuilder::visitCallBr.
>
> It looks like the problem is that we split the "block" into two MBBs, and the successors end up attached to the second block, instead of the first.  Not sure where that happens, off the top of my head.

I think this is what's going on in `ScheduleDAGSDNodes::EmitSchedule`; the results of the before/after split don't look quite right in terms of pred/succ lists, but I've got to dig more into this tomorrow.

>> Maybe blockaddress Constants could be generated as late as possible. Instead, we'd pass BasicBlock operands around in the Instruction level IR, then lower to MachineBasicBlock operands at the MachineInstr level, then only generate blockaddress operands very late when lowering to MCInst level?
> 
> I think the interaction between blockaddresses and indirectbr is fine.  The indirection of blockaddress constants is actually helpful when you're dealing with random instructions that aren't indirectbr/callbr.
> 
> For callbr, the indirection isn't so helpful, so yes, it would make sense to just use BasicBlock/MachineBasicBlock directly.

That's a yak shave for another day, but one I really want to do since I think it makes `callbr` and transforms on it less brittle.  (Particularly ones that use get/setOperand to modify one operand, when technically they should be modifying two).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76961