[llvm] [MachineLateInstrsCleanup] Handle multiple kills for a preceding definition. (PR #119132)
Jonas Paulsson via llvm-commits
llvm-commits at lists.llvm.org
Tue Dec 10 07:42:40 PST 2024
JonPsson1 wrote:
> You could also just do MRI.clearKillFlags any time it could matter
I tried this now, and found that
- Still need to update live-in lists recursively: At first I just added Reg as live-in to the current MBB which the verifier accepted (it would have been better if it could report a missing definition for a live-out register in the predecessor). It however became clear that with the CFG optimizations running later on, all the predecessors up the CFG need to have the live-in list updated.
- It's still a nice simplification to the algorithm (to take a look, see HEAD^ on the branch / dc4ff43).
- Practically NFC on SPEC / SystemZ: only 7 files change without any real impact. This seemed due to BranchFolding.cpp mostly, and also MachineCopyPropagation in one case.
- Only two CodeGen tests fail, however CodeGen/Thumb/frame-access.ll crashes with an assertion, which was unexpected:
```
MachineOperand::isKill() const: Assertion `isReg() && "Wrong MachineOperand accessor"'
bool ARMLoadStoreOpt::CombineMovBx(MachineBasicBlock &MBB) {
MachineBasicBlock::iterator MBBI = MBB.getFirstTerminator();
if (MBBI == MBB.begin() || MBBI == MBB.end() ||
MBBI->getOpcode() != ARM::tBX_RET)
return false;
MachineBasicBlock::iterator Prev = MBBI;
--Prev;
if (Prev->getOpcode() != ARM::tMOVr ||
!Prev->definesRegister(ARM::LR, /*TRI=*/nullptr))
return false;
for (auto Use : Prev->uses())
=> if (Use.isKill()) {
(gdb) p Prev->dump()
$lr = frame-destroy tMOVr $r1, 14, $noreg
(gdb) p Use.dump()
14
```
It seems there is some assumption there that some register operand must be a kill.
- There is also ReachingDefAnalysis.cpp that checks for kills, which is used in BreakFalseDeps.cpp (only used by ARM / X86 at the moment).
Due to the above and in particular the crash in ARMLoadStoreOpt (which has a lot of checks for kills), I reverted back to the tracking of kill flags and clearing them carefully as per before. I would agree that there is room for improvement here, and at least on SystemZ it seems completely fine to remove any and all kill flags after regalloc.
https://github.com/llvm/llvm-project/pull/119132
More information about the llvm-commits
mailing list