This is an archive of the discontinued LLVM Phabricator instance.

[AVR][Clang] Implement __AVR_HAVE_*__ macros
ClosedPublic

Authored by aykevl on Nov 7 2022, 9:54 AM.

Details

Summary

These macros are defined in avr-gcc and are useful when working with assembly.
For example, startup code needs to copy the contents of .data from flash to RAM, but should use elpm (instead of lpm) on devices with more than 64kB flash. Without __AVR_HAVE_ELPM__, there is no way to know whether the elpm instruction is supported.

Depends on D137521.

Diff Detail

Event Timeline

aykevl created this revision.Nov 7 2022, 9:54 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 7 2022, 9:54 AM
Herald added a subscriber: Jim. · View Herald Transcript
aykevl requested review of this revision.Nov 7 2022, 9:54 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 7 2022, 9:54 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
benshi001 added inline comments.Nov 11 2022, 2:03 AM
clang/lib/Basic/Targets/AVR.cpp
355

ATxmega16a4 with family code 102 also supports ELPM. Could you please make a careful check on ELPM and all other features?

Generally speaking I am very glad to have this patch committed, since it fixes

https://github.com/llvm/llvm-project/issues/56157

benshi001 added inline comments.Nov 11 2022, 2:28 AM
clang/lib/Basic/Targets/AVR.cpp
355

I suggest that you can make your code in accordance with https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/AVR/AVRDevices.td

benshi001 requested changes to this revision.Nov 11 2022, 2:28 AM
This revision now requires changes to proceed.Nov 11 2022, 2:28 AM
aykevl updated this revision to Diff 475419.Nov 15 2022, 4:14 AM
  • Fix ArchHas3BytePC to remove arch 107 (of which no chips have more than 128kB flash)
  • Fix ArchHasELPM/ArchHasELPMX to add arch 102 (it does support elpm even though avr-gcc claims it doesn't).
  • Add notes where the provided macros may not be correct in all cases. It's still better than avr-gcc though.
aykevl added inline comments.Nov 15 2022, 4:25 AM
clang/lib/Basic/Targets/AVR.cpp
355

I checked all 102 family chips and indeed they all support ELPM. This looks like a bug in avr-gcc, which claims these devices don't support ELPM.

I did check AVRDevices.td but apparently I made some mistakes. I've checked again and fixed a few small issues. There are still some left but they are difficult to fix without reading AVRDevices.td directly.

benshi001 accepted this revision.Nov 17 2022, 10:54 PM

LGTM

clang/lib/Basic/Targets/AVR.cpp
355

That is OK enough, Thanks.

This revision is now accepted and ready to land.Nov 17 2022, 10:54 PM
This revision was landed with ongoing or failed builds.Nov 22 2022, 4:22 PM
This revision was automatically updated to reflect the committed changes.