This is an archive of the discontinued LLVM Phabricator instance.

[PowerPC] Add handling for WACC register spilling.
ClosedPublic

Authored by stefanp on Oct 25 2022, 6:32 PM.

Details

Summary

This patch adds spilling for the new WACC registers.

In order to get the spilling test to work the MMA instructions from Power 10 are
now supported for Future CPU except that they are all using the new WACC
registers instead of the ACC registers from Power 10.

Diff Detail

Event Timeline

stefanp created this revision.Oct 25 2022, 6:32 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 25 2022, 6:32 PM
stefanp requested review of this revision.Oct 25 2022, 6:32 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 25 2022, 6:32 PM
stefanp added a reviewer: Restricted Project.Oct 25 2022, 6:32 PM
amyk added inline comments.Oct 28 2022, 3:09 PM
llvm/lib/Target/PowerPC/PPCInstrMMA.td
243

A super minor nit, but I think the indentation on the lines below the def are off by 1.

546

Are able to elaborate on why?

570

nit: Indentation.

574

nit: Indentation.

llvm/lib/Target/PowerPC/PPCRegisterInfo.cpp
580
1301

Realized I had a question about this and just wanted to confirm - but the reason we are using dbgs() instead of the usual LLVM_DEBUG(dbgs()) is because of the #ifdef NDEBUG, right? Meaning in the cases where we're using the macro, we shouldn't be doing it the LLVM_DEBUG() way?

stefanp updated this revision to Diff 473101.Nov 3 2022, 7:07 PM

Fixed some spacing and a couple of comments.

llvm/lib/Target/PowerPC/PPCRegisterInfo.cpp
1301

Yes that's correct.
Here is how LLVM_DEBUG is defined.

#ifdef NDEBUG
#define LLVM_DEBUG(X)
#else
#define LLVM_DEBUG(X) do { if (DebugFlag) { X; } } while (0)
#endif

The idea is that if we have NDEBUG defined then LLVM_DEBUG is defined as doing nothing. If we use #ifdef NDEBUG directly in code then we know that in the else case the code will be executed in debug mode and it doesn't need to be guarded with LLVM_DEBUG because it is already in the else case of the NDEBUG.

saghir accepted this revision as: saghir.Nov 11 2022, 11:33 AM
saghir added a subscriber: saghir.

LGTM.

This revision is now accepted and ready to land.Nov 11 2022, 11:33 AM
amyk accepted this revision as: amyk.Nov 17 2022, 7:21 PM

Thanks for addressing my comments and question, Stefan. I think this overall LGTM.

stefanp updated this revision to Diff 477185.Nov 22 2022, 7:36 AM

Rebased to top of main branch.

This revision was landed with ongoing or failed builds.Nov 22 2022, 7:38 AM
This revision was automatically updated to reflect the committed changes.