Page MenuHomePhabricator

[CodeGen][RegAllocFast] Add MRI delegate callback to notify VReg spill
AbandonedPublic

Authored by cdevadas on Sep 30 2022, 4:55 AM.

Details

Summary

Add a callback to notify targets whenever a virtual register is
spilled/restored during fast regalloc. The physical register is
directly spilled in case of RegAllocFast and targets sometimes
need to track the spills for certain virtual registers involved
in special operations.

Also, used cloneVirtualRegister in LiveIntervals while cloning
a virtual register.

Diff Detail

Event Timeline

cdevadas created this revision.Sep 30 2022, 4:55 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 30 2022, 4:55 AM
cdevadas requested review of this revision.Sep 30 2022, 4:55 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 30 2022, 4:55 AM

Testcase added with D134951
llvm/test/CodeGen/AMDGPU/wwm-register-spill-during-regalloc.ll

Hi,

The LiveIntervals and LiveRangeEdit changes make sense to me (though in practice they are NFC.)

However, I'm not a fan of the MachineRegisterInfo change. Since it sounds like you need that field only for fast regalloc, could you do something specifically in that pass/with the related target hooks in that pass?

Cheers,
-Quentin

However, I'm not a fan of the MachineRegisterInfo change. Since it sounds like you need that field only for fast regalloc, could you do something specifically in that pass/with the related target hooks in that pass?

The alternative would be to pass the flags as an argument to the target hooks storeRegToStackSlot & loadRegFromStackSlot which are common to both fast regalloc and greedy.

cdevadas updated this revision to Diff 470474.Oct 25 2022, 7:12 AM
cdevadas retitled this revision from [CodeGen] Propagate vreg flags to the regalloc spiller interface to [CodeGen][RegAllocFast] Add MRI delegate callback to notify VReg spill.
cdevadas edited the summary of this revision. (Show Details)

Added a new delegate callback to notify targets whenever a virtual register is spilled/restored during RegAllocFast.

cdevadas updated this revision to Diff 470711.Oct 25 2022, 10:34 PM

Removed the enum class DelegateReceiver and changed the code to notify all delegates when an event occurs.

cdevadas updated this revision to Diff 470902.Oct 26 2022, 12:38 PM

Rebase after recent changes in D134950.

qcolombet accepted this revision.Oct 26 2022, 2:01 PM

LGTM.

llvm/lib/CodeGen/RegAllocFast.cpp
494

Nit: we may want to have two different events for spill and reload. But I'm fine keeping this as is for now if that is enough for your needs.

This revision is now accepted and ready to land.Oct 26 2022, 2:01 PM
arsenm added inline comments.Oct 27 2022, 8:03 AM
llvm/include/llvm/CodeGen/MachineRegisterInfo.h
189

I assume TheDelegates are pointers, so const X *TheDelegate?

llvm/lib/CodeGen/LiveRangeEdit.cpp
36

Is the clone still strictly necessary for this patch?

cdevadas added inline comments.Oct 27 2022, 10:24 AM
llvm/include/llvm/CodeGen/MachineRegisterInfo.h
189

Yes, they are pointers, will use *

llvm/lib/CodeGen/LiveRangeEdit.cpp
36

Not necessarily in the same patch. They are basically needed to notify the targets when the RegAllocFast clone registers (for AMDGPU they are needed to propagate the vreg flags). They are included in the earlier patch where the VRegFlags are added directly in the MachineRegisterInfo file.

cdevadas updated this revision to Diff 471224.Oct 27 2022, 10:38 AM

Used auto *Delegate in the range-based loop.

cdevadas abandoned this revision.Dec 23 2022, 11:37 PM

D138656 handled it better and hence abanding it.