add a new goal MustReduceRegisterPressure for machine combiner pass. This is an NFC patch.
PowerPC will use this new goal to do some register pressure related optimization. https://reviews.llvm.org/D92071
Paths
| Differential D92068
[MachineCombiner] [NFC]Add MustReduceRegisterPressure goal ClosedPublic Authored by shchenz on Nov 24 2020, 7:40 PM.
Details
Summary add a new goal MustReduceRegisterPressure for machine combiner pass. This is an NFC patch. PowerPC will use this new goal to do some register pressure related optimization. https://reviews.llvm.org/D92071
Diff Detail
Unit TestsFailed Event Timelineshchenz added a child revision: D92071: [PowerPC] support register pressure reduction in machine combiner..Nov 24 2020, 8:00 PM shchenz edited reviewers, added: Restricted Project, efriedma; removed: eli.friedman.Nov 26 2020, 9:46 PM Comment Actions 1: fix lint warning messages and gentle ping... shchenz added a parent revision: D92557: [MachineLICM] delete dead flag if the duplicated def outside of loop is dead.. Comment Actions I'm not too familiar with MIR and I have not worked on MachineCombiner in a long time, so someone else should probably have a look too.
Comment Actions @spatel Very much appreciate your comments. It is a good start for this patch to move on.
For the dead flag in case test/DebugInfo/X86/machinecse-wrongdebug-hoist.ll, I think now it is more accurate. It should be an improvement. static bool isOperandKill(const MachineOperand &MO, MachineRegisterInfo *MRI) { return MO.isKill() || MRI->hasOneNonDBGUse(MO.getReg()); } yeah, sure, if this is a must fix degradation, I will fix this in follow up patches.
Compile-time diff: https://llvm-compile-time-tracker.com/compare.php?from=1bb79875e4b8f9018142a5155ca3f7df37778419&to=16f8136e47fdf13a05a27bbd0829d670f3baacb1&stat=instructions Thanks again.
Comment Actions
I don't know the answer to that question - does it potentially harm other passes if the kill flag was dropped, or can we assume it does not matter? Adding more potential reviewers.
I assume it's an x86 machine, but @nikic can confirm.
We have tried to avoid changes that can cost that much compile-time without any benefit. If every target will have that loss even if there is no chance of run-time improvement, we should look for ways to hide it. Again, I'm not very familiar with how things work here - is it possible to make the extra analysis conditional on some flags internal to the machine combiner pass? Comment Actions I agree, this shouldn't incur compile-time cost for targets on which this won't even be run. Comment Actions Thanks for your comments @spatel @lebedev.ri Do you have any idea to run the LiveIntervals pass only on the target that MustReduceRegisterPressure is enabled? shchenz retitled this revision from [MachineCombiner] Add MustReduceRegisterPressure goal to [MachineCombiner] [NFC]Add MustReduceRegisterPressure goal. Comment Actions1: don't require LiveIntervals analysis shchenz removed a parent revision: D92557: [MachineLICM] delete dead flag if the duplicated def outside of loop is dead..Dec 10 2020, 3:52 AM Comment Actions Hi @spatel @lebedev.ri , I changed the patch not requiring LiveIntervals pass. Now this patch is NFC. New compiling time diff is: https://llvm-compile-time-tracker.com/compare.php?from=5bb28fa0f51e94522644fe5633877b441b9ad8d3&to=73b8523c4c149866690257f58bc678be20a4991b&stat=instructions Could you help to have another look? Thanks. Comment Actions If this is NFC now, no objections from me - I just noted minor errors.
Comment Actions
Thanks for your careful review. Updated accordingly. Thanks again for making this patch go so far. This revision is now accepted and ready to land.Dec 12 2020, 9:05 AM This revision was landed with ongoing or failed builds.Dec 13 2020, 9:03 PM Closed by commit rG4830d458dd0d: [MachineCombiner][NFC] Add MustReduceRegisterPressure goal (authored by shchenz). · Explain Why This revision was automatically updated to reflect the committed changes.
Revision Contents
Diff 309192 llvm/include/llvm/CodeGen/TargetInstrInfo.h
llvm/lib/CodeGen/MachineCombiner.cpp
llvm/lib/CodeGen/TargetInstrInfo.cpp
llvm/lib/Target/AArch64/AArch64InstrInfo.h
llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
llvm/lib/Target/PowerPC/PPCInstrInfo.h
llvm/lib/Target/PowerPC/PPCInstrInfo.cpp
llvm/test/CodeGen/AArch64/O3-pipeline.ll
llvm/test/CodeGen/AArch64/aarch64-combine-fmul-fsub.mir
llvm/test/CodeGen/AArch64/machine-combiner-instr-fmf.mir
llvm/test/CodeGen/PowerPC/opt-cmp-inst-cr0-live.ll
llvm/test/CodeGen/X86/opt-pipeline.ll
llvm/test/DebugInfo/X86/machinecse-wrongdebug-hoist.ll
|
typo: "if target supports reassociation of instructions"