This is an archive of the discontinued LLVM Phabricator instance.

[AVR] Fix incorrect source regclass of LDWRdPtr
ClosedPublic

Authored by Jim on May 23 2019, 3:46 AM.

Details

Summary

LDWRdPtr would be expanded to ld+ldd. ldd only accepts the pointer register is Y or Z.
So the register class of pointer of LDWRdPtr should be PTRDISPREGS instead of PTRREGS.

Diff Detail

Repository
rL LLVM

Event Timeline

Jim created this revision.May 23 2019, 3:46 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 23 2019, 3:46 AM
dylanmckay added inline comments.May 26 2019, 10:33 PM
lib/Target/AVR/AVRInstrInfo.td
1163 ↗(On Diff #200923)

Thanks for fixing this comment

lib/Target/AVR/AVRRegisterInfo.td
175 ↗(On Diff #200923)

I can't tell why the removal of Y is needed.

LDWRdPtr expands to LDRdPtr and LDDRdPtrQ.

LDRdPtr accepts any pointer register (X, Y, or Z), and as you point out, LDDRdPtrQ only accepts DREGS_WITHOUT_Z_WORKAROUND: (AKA some GPRs and X, Y). The register constraints over the instruction pair is the intersection of the register classes for these sub instructions, which is the set of pointer registers { X, Y }

What are your thoughts?

Jim marked 2 inline comments as done.May 27 2019, 12:24 AM
Jim added inline comments.
lib/Target/AVR/AVRRegisterInfo.td
168 ↗(On Diff #200923)

Since we change the register class of pointer for LDWRdPtr to the small one . We have the same issue like Z register.

175 ↗(On Diff #200923)

LDRdPtr accepts X, Y or Z as pointer register, But LDDRdPtrQ only accepts Y or Z as pointer register.
I changed the pointer register of LDWRdPtr from {X, Y, Z} to {Y, Z}. Since their intersection of registers are Y or Z (Register class PTRDISPREGS). Y has more register pressure due to this change. So I removed Y from DREGS_WITHOUT_Z_WORKAROUND. In my experiment, if it is without the removal of Y, it has the same issue, ran out of registers, as Z.

test/CodeGen/AVR/pseudo/LDDWRdPtrQ-same-src-dst.mir
1 ↗(On Diff #200923)

The source register and pointer register of LDDWRdPtrQ will not be the same. So I deleted this test.

dylanmckay added inline comments.Jun 1 2019, 1:31 AM
lib/Target/AVR/AVRRegisterInfo.td
175 ↗(On Diff #200923)

That makes sense, thanks

dylanmckay accepted this revision.Jun 1 2019, 1:32 AM

Nice work, thanks for the patch @Jim

This revision is now accepted and ready to land.Jun 1 2019, 1:32 AM
This revision was automatically updated to reflect the committed changes.
llvm/trunk/test/CodeGen/AVR/load.ll