This is an archive of the discontinued LLVM Phabricator instance.

[AVR] Fix I/O instructions on XMEGA
ClosedPublic

Authored by vlastik on Apr 1 2020, 3:11 AM.

Details

Summary

On XMEGA, I/O address space is same as data address space - there is no 0x20 offset,
because CPU General Purpose Registers are not mapped in data address space.

From https://en.wikipedia.org/wiki/AVR_microcontrollers

In the XMEGA variant, the working register file is not mapped into the data address space; as such, it is not possible to treat any of the XMEGA's working registers as though they were SRAM. Instead, the I/O registers are mapped into the data address space starting at the very beginning of the address space.

Diff Detail

Event Timeline

vlastik created this revision.Apr 1 2020, 3:11 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 1 2020, 3:11 AM
vlastik updated this revision to Diff 254146.Apr 1 2020, 3:15 AM

Fixed 16bit accesses too

Nice work, I didn't know of this particular XMEGA quirk.

I left a couple comments, then it looks good to merge.

Could you also add a CodeGen test to test/CodeGen/AVR/features for an XMEGA chip, along with a CHECK line that verifies that the correct assembly output is generated for XMEGA?

llvm/lib/Target/AVR/AVRDevices.td
127

add an extra s to addres

llvm/lib/Target/AVR/AVRInstrInfo.td
110–112

Rather than hardcoding these magic constants in this file, recommend adding a new method to AVRSubtarget, like uint8_t getIoRegisterOffset() and moving the ternary logic there.

Reuse this method in the below PatLeafs

vlastik updated this revision to Diff 256974.Apr 13 2020, 7:52 AM

Fixed typo (missing s in address)
Moved magic logic to function in AVRSubtarget
Implemented a CodeGen test

vlastik marked 2 inline comments as done.Apr 13 2020, 7:56 AM
dylanmckay accepted this revision.Apr 18 2020, 7:55 PM

Really nice work, it's good to go.

This revision is now accepted and ready to land.Apr 18 2020, 7:55 PM

Really nice work, it's good to go.

Thanks, then I need someone to commit the patch, please

@vlastik Merged into master in 1420f4efbe7ca34355b2dd85d396197d92cd5e3f.

Thanks again for the patch!

This revision was automatically updated to reflect the committed changes.
RKSimon added inline comments.
llvm/lib/Target/AVR/AVRInstrInfo.td
146

@vlastik the "val >= 0x0" is redundant as these are unsigned values and gcc warns with:

lib/Target/AVR/AVRGenDAGISel.inc:1676:14: error: comparison of unsigned expression >= 0 is always true [-Werror=type-limits]
   return val >= 0x0 && val < 0x3f;

which causes problems with Werror builds - can these be removed please?

RKSimon added inline comments.May 21 2020, 8:02 AM
llvm/lib/Target/AVR/AVRInstrInfo.td
146