Details

Reviewers

Commits

rG9757f4f2ddb7: [PowerPC] Don't use the S30 and S31 regs for the pic code

Summary

These changes to address the https://github.com/llvm/llvm-project/issues/55857 issue .

Solution proposed was , Since R30/S30 used as pointer (32 bits ) for GOT Table in the ppc32 ABI ,so remove from the SPE callee save register when pic enabled .

So you don't emit the spe load and store inst for S30 and S31 regs.

Diff Detail

Repository: rG LLVM Github Monorepo

Event Timeline

umesh.kalappa0 created this revision.Jun 10 2022, 6:15 AM

umesh.kalappa0 created this object with visibility "All Users".

Herald added a project: Restricted Project. · View Herald TranscriptJun 10 2022, 6:15 AM

Herald added subscribers: kbarton, hiraditya. · View Herald Transcript

umesh.kalappa0 requested review of this revision.Jun 10 2022, 6:15 AM

Herald added a subscriber: llvm-commits. · View Herald TranscriptJun 10 2022, 6:15 AM

umesh.kalappa0 updated this revision to Diff 435895.Jun 10 2022, 6:26 AM

chmeee added inline comments.Jun 10 2022, 7:19 AM

llvm/lib/Target/PowerPC/PPCCallingConv.td
280	Should we also drop S31? R31 is the frame pointer typically, so should almost never be an SPE register in the ABI.

Harbormaster completed remote builds in B169060: Diff 435895.Jun 10 2022, 7:42 AM

umesh.kalappa0 updated this revision to Diff 435929.Jun 10 2022, 8:36 AM

umesh.kalappa0 retitled this revision from Don't use the S30 reg for the pic code . to Don't use the S30 and S31 regs for the pic code ..

umesh.kalappa0 edited the summary of this revision. (Show Details)

umesh.kalappa0 marked an inline comment as done.

umesh.kalappa0 added inline comments.

llvm/lib/Target/PowerPC/PPCCallingConv.td
280	Agree and updated the changes.

I'm not sure of the proper use of S31 in general, since ABI makes r31 the frame pointer in general. If we stick with only supporting the ABI, and not supporting alternatives, I think S31 should just be removed in general, unless we support omitting the frame pointer.

But I think in general S31 shouldn't be conditional on PIC, since its use is orthogonal. Maybe a different change?

llvm/lib/Target/PowerPC/PPCCallingConv.td
275	Should S31 be dropped up here, too? Or should this remain as the list of all callee saved registers?

Harbormaster completed remote builds in B169083: Diff 435929.Jun 10 2022, 9:46 AM

umesh.kalappa0 updated this revision to Diff 436193.Jun 11 2022, 10:41 PM

umesh.kalappa0 marked an inline comment as done.

umesh.kalappa0 added inline comments.

llvm/lib/Target/PowerPC/PPCCallingConv.td
275	Taken care

Harbormaster completed remote builds in B169287: Diff 436193.Jun 11 2022, 11:27 PM

umesh.kalappa0 marked an inline comment as done.Jun 12 2022, 8:55 PM

@chmee and @nemanjai . ,Please do you have any other comments or ok to commit the same ?

chmeee accepted this revision.Jun 14 2022, 6:49 AM

This revision is now accepted and ready to land.Jun 14 2022, 6:49 AM

umesh.kalappa0 removed a reviewer: nemanjai.Jun 14 2022, 7:50 AM

Herald added a subscriber: nemanjai. · View Herald TranscriptJun 14 2022, 7:50 AM

@chmeee ,Can you commit for us ,since we don't have the commit access for the repo.

In D127495#3581809, @umesh.kalappa0 wrote:

@chmeee ,Can you commit for us ,since we don't have the commit access for the repo.

Sorry, but my computer with the relevant keys is in storage for a pending move, so I won't be able to commit for a month or so. @nemanjai is better positioned to do it.

In D127495#3582591, @chmeee wrote:

In D127495#3581809, @umesh.kalappa0 wrote:

@chmeee ,Can you commit for us ,since we don't have the commit access for the repo.

Sorry, but my computer with the relevant keys is in storage for a pending move, so I won't be able to commit for a month or so. @nemanjai is better positioned to do it.