[PATCH] D79635: [RISCV] Split the pseudo instruction splitting pass

[Sam Elliott via Phabricator via llvm-commits]

lenary added a comment.

@luismarques yep, you've got my analysis correct.

I think, last time I looked at it, `MachineBasicBlock`s had some weirdness associated with `hasAddressTaken`, as @rogfer01 points out. They used to require the respective IR BasicBlock also had its address taken, as is not the case with these labels. Looking at `llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:3030` this is no longer the case, but git-blame tells me that hasn't changed for years, so I must be misremembering something on re-investigation.

The broader point is also correct though: lots of passes which check if a MBB has its address taken, don't also check if the label must be emitted, which this code "assumes". Updating these passes isn't always going to be easy.

I think for the moment the NFC is to keep these passes together (as @luismarques has done), and later we can choose a different approach (and/that lets us move the non-atomic pseudo expansion earlier). I think MachineInstr can now have associated labels that don't require the basic block to be split, as is going on here. There are notes that API is not yet entirely complete, though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79635