This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Refine getMaxPushPopReg like getLibCallID. NFC.
ClosedPublic

Authored by Jim on Jul 26 2023, 10:30 PM.

Details

Summary

If save/restore libcall or Zcmp push/pop are enabled, callee-saved registers including ra would be
assigned a negative frame index (-1 for ra, -2 for s0, ..., -13 for s11) by RISCVRegisterInfo::hasReservedSpillSlot.

getLibCallID would find a callee-saved register that has max register id and with assigned negative frame index.
And use this max register to get which save/restore libcall should be selected.

In getMaxPushPopReg (for Zcmp push/pop, similar getLibCallID), it uses extra register class (PGPR) to
check the saved register is in ra, s0-s11 instead of checking the frame index is negative.

This patch just changes the checking for the saved register from is contained in PGPR to has a negative frame index.

Diff Detail

Unit TestsFailed

Event Timeline

Jim created this revision.Jul 26 2023, 10:30 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 26 2023, 10:30 PM
Jim requested review of this revision.Jul 26 2023, 10:30 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 26 2023, 10:30 PM
Jim added a comment.Jul 26 2023, 11:00 PM

testcase?

It is hard to add testcase to show the differece. It only reuses the assumption that register in ra-s11 would be assigned a negative frame index
by RISCVRegisterInfo::hasReservedSpillSlot instead of adding a new register class PGPR to check register inside ra-s11.

I guess I am confused, does it fix some bug or just kind of refine?

I have no clue either from skimming this, the patch summary is extremely unclear and really hard to parse as English.

jrtc27 requested changes to this revision.Jul 26 2023, 11:20 PM

Needs a clear summary at least

This revision now requires changes to proceed.Jul 26 2023, 11:20 PM
fakepaper56 added inline comments.Jul 26 2023, 11:43 PM
llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
270–273

indexes -> indices.
by Push -> by Zcmp Push.

Jim edited the summary of this revision. (Show Details)Jul 27 2023, 12:00 AM
Jim updated this revision to Diff 544655.Jul 27 2023, 1:50 AM
Jim edited the summary of this revision. (Show Details)

Address @fakepaper56's comment.

Jim marked an inline comment as done.Jul 27 2023, 1:51 AM
Jim retitled this revision from [RISCV] Fix getMaxPushPopReg like getLibCallID to [RISCV] Refine getMaxPushPopReg like getLibCallID.Jul 27 2023, 2:01 AM

So actually it's a NFC?

Jim retitled this revision from [RISCV] Refine getMaxPushPopReg like getLibCallID to [RISCV] Refine getMaxPushPopReg like getLibCallID. NFC..Jul 27 2023, 2:15 AM

So actually it's a NFC?

Yes, it is. Append NFC to title of this revision.

Jim updated this revision to Diff 547633.Aug 6 2023, 8:14 PM

Rebase

This revision was not accepted when it landed; it landed in state Needs Review.Aug 7 2023, 12:16 AM
This revision was automatically updated to reflect the committed changes.