[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