This is an archive of the discontinued LLVM Phabricator instance.

[PowerPC] Custom lower known CR bit spills
ClosedPublic

Authored by lei on May 9 2019, 12:38 PM.

Details

Summary

For known CRBit spills, CRSET/CRUNSET, it is more efficient to just load and spill the known value instead of extracting the bit.

eg. This sequence is currently used to spill a CRUNSET:

crclr   4*cr5+lt
mfocrf  r3,4
rlwinm  r3,r3,20,0,0
stw     r3,132(r1)

This patch custom lower it to:

li  r3,0
stw r3,132(r1)

Diff Detail

Repository
rL LLVM

Event Timeline

lei created this revision.May 9 2019, 12:38 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 9 2019, 12:38 PM
hfinkel added inline comments.May 9 2019, 2:39 PM
llvm/lib/Target/PowerPC/PPCRegisterInfo.cpp
725

I think that this makes sense, but I'm a bit concerned that, without a cutoff, this makes the spilling process quadratic. Can you please add a cl::opt search cutoff for this?

lei marked an inline comment as done.May 10 2019, 12:47 PM
lei added inline comments.
llvm/lib/Target/PowerPC/PPCRegisterInfo.cpp
725

How about --ppc-max-crbit-spill-dist with an initial value of 20?

hfinkel added inline comments.May 10 2019, 1:25 PM
llvm/lib/Target/PowerPC/PPCRegisterInfo.cpp
725

Sounds good. Make the initial value a bit larger, however. It's easy to have blocks with more than 20 instructions. I'd start with 100. Also, remember to skip the debug instructions when counting, so you don't end up with differences between the debugging-enabled and debugging-disabled cases.

lei marked an inline comment as done.May 10 2019, 3:33 PM
lei added inline comments.
llvm/lib/Target/PowerPC/PPCRegisterInfo.cpp
725

I didn't even think about debug instructions... how do I identify if an instruction is a debug instruction?

hfinkel added inline comments.May 13 2019, 8:56 AM
llvm/lib/Target/PowerPC/PPCRegisterInfo.cpp
725

You can, I believe, call Ins->isDebugInstr()

lei updated this revision to Diff 199312.May 13 2019, 12:43 PM
lei marked an inline comment as done.

Added option to specify cutoff for CR bit definition search with default of 100.

hfinkel added inline comments.May 13 2019, 12:56 PM
llvm/lib/Target/PowerPC/PPCRegisterInfo.cpp
725

Please put spaces around the = here, and the = and == below (to match the general convention here).

lei updated this revision to Diff 199318.May 13 2019, 1:10 PM
lei marked an inline comment as done.

address spacing issues.

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

thx!

hfinkel accepted this revision.May 13 2019, 3:10 PM

LGTM.

If this test case proves too fragile in the future, we might replace it with an MIR test.

This revision is now accepted and ready to land.May 13 2019, 3:10 PM
lei added a comment.May 14 2019, 6:45 AM

LGTM.

If this test case proves too fragile in the future, we might replace it with an MIR test.

Hi Hal, I actually tried to create a MIR test first but had a hard time. I was trying to get the MIR prior to my pass via llc -stop-before=regalloc reduced.ll but it gave me LLVM ERROR: "regalloc" pass is not registered.. I just stopped there as I didn't know how to continue after that... let me know if you have suggestions on how to proceed with this kind of issue and I'll try it next time. Thx!

This revision was automatically updated to reflect the committed changes.