Page MenuHomePhabricator

[PowerPC] Fix VSX clobbers of CSR registers
AcceptedPublic

Authored by nemanjai on Oct 7 2019, 7:28 AM.

Details

Reviewers
hfinkel
lei
jsji
Group Reviewers
Restricted Project
Summary

If an inline asm statement clobbers a VSX register that overlaps with a callee-saved Altivec register or FPR, we will not record the clobber and will therefore violate the ABI. This is clearly a bug so this patch fixes it.

Diff Detail

Repository
rL LLVM

Event Timeline

nemanjai created this revision.Oct 7 2019, 7:28 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 7 2019, 7:28 AM
amyk added a subscriber: amyk.Oct 11 2019, 10:44 PM
amyk added inline comments.
test/CodeGen/PowerPC/inline-asm-vsx-clobbers.ll
2

Should we add -verify-machineinstrs to the test case?

nemanjai marked an inline comment as done.Mon, Oct 28, 6:07 PM
nemanjai added inline comments.
test/CodeGen/PowerPC/inline-asm-vsx-clobbers.ll
2

Yup. We should. I'll add it on the next revision.

lei accepted this revision.Tue, Nov 12, 7:18 AM
lei added a subscriber: lei.

LGTM.
Please address Amy's comment on commit.

This revision is now accepted and ready to land.Tue, Nov 12, 7:18 AM
jsji accepted this revision.Tue, Nov 12, 2:06 PM

LGTM as well.

lib/Target/PowerPC/PPCISelLowering.cpp
14320

Do we need to consider 32 or 64 bits regclass here? If not, maybe we should add some comments to explain?

test/CodeGen/PowerPC/inline-asm-vsx-clobbers.ll
3

Why we need -enable-ppc-quad-precision here?

nemanjai marked 2 inline comments as done.Fri, Nov 15, 1:13 PM

Thanks for your comments. I'll address them and commit this.

lib/Target/PowerPC/PPCISelLowering.cpp
14320

I believe you mean VSSRC/VSFRC (i.e. the scalar VSX registers). I believe those have to be named as f<N> or vs<N> in the clobbers list so we should still handle them correctly.

test/CodeGen/PowerPC/inline-asm-vsx-clobbers.ll
3

Ha ha, good catch. That's just an errant option. I'll remove it on the commit.