AVR port now supports only ATmega328.
This patch makes more MCUs work with clang
Details
Details
- Reviewers
dylanmckay rjmccall MaskRay
Diff Detail
Diff Detail
Unit Tests
Unit Tests
| Time | Test | |
|---|---|---|
| 190 ms | Clang.Driver::Unknown Unit Message ("") | |
| 400 ms | Clang.Misc::Unknown Unit Message ("") |
Event Timeline
Comment Actions
Fix regression introduced by patch
Failed unit tests:
- Clang.Driver::avr-link-mcu-family-unimplemented.c
- Clang.Misc::target-invalid-cpu-note.c
| clang/lib/Basic/Targets/AVR.h | ||
|---|---|---|
| 21 | Order this #include alphabetically with the ones above it | |
| llvm/include/llvm/Support/AVRTargetParser.h | ||
| 10 | Add another sentence or two to this documentation, as this will be a new public-facing API that all frontends can use. | |
| llvm/lib/Support/AVRTargetParser.cpp | ||
| 21 | Add a comment like FIXME: At some point, this list should removed and the logic directly driven from the definitions in AVRDevices.td`. | |
Comment Actions
Great! I was considering writing something like this but this patch is much better than what I had in mind.
Note that this might conflict with D78117.
Comment Actions
Patch generally looks good, but I agree that it could use some basic tests. You don't need to do specifically test all 20 million releases, but make sure you cover the first and last in the array as well as something in the middle, just to make sure your scans are working right.
| llvm/include/llvm/Support/AVRTargetParser.h | ||
|---|---|---|
| 28 | I think the relationship would be clearer if this type were named MCUFamilyInfo. More generally, these types and fields could use some comments, like "Information about a specific AVR microcontroller release" or "Information about a family of AVR microcontrollers" or "Some sort of number that goes somewhere and has some kind of meaning, maybe it's reported by hardware or something, I dunno". | |
Comment Actions
A thought: should the AVR family be included in the target triple? Like avr5-unknown-unknown (similar to armv6m-none-eabi, armv7m-none-eabi, etc).
The different variations do sometimes change the architecture in incompatible ways, such as with the call instruction which pushes a return address of two or three bytes depending on the architecture variant (and thus makes it impossible to pass parameters on the stack reliably).
I don't think this should block this patch (which I hope will get updated so it can land soon), but it may be something to consider.
Comment Actions
A thought: should the AVR family be included in the target triple? Like avr5-unknown-unknown (similar to armv6m-none-eabi, armv7m-none-eabi, etc).
The different variations do sometimes change the architecture in incompatible ways, such as with the call instruction which pushes a return address of two or three bytes depending on the architecture variant (and thus makes it impossible to pass parameters on the stack reliably).
I've thought about this a few years ago, I suspect it is not possible. It seems like we really do need to compile a sysroot/stdlib/runtime libraries for each and every distinct MCU.
https://github.com/rust-lang/rust/issues/44036#issuecomment-324330757
Does each device have a subtly different ISA?
tl;dr Sometimes, yes
In general, all devices can be _more or less_ grouped into families.
For example, avr1, avr2, avr25, ... avr5, avr51, are all family names. These families each define an subset of the full AVR ISA.
You can find the AVR backend's definition of these families in AVRDevices.td. You can see that in general, the families build up on top of each other in terms of supported instructions/features.
The problem is that not every realised ISA can be cleanly separated into a family.
For example, the ATtiny26 is a part of the avr2 family, but it also happens to support the "LPMX" set of instructions
Another example is the ATMega8515, which _almost_ implements all of avr4, but not quite all, and so the linked definition bases the device off avr2 and adds the extra features explicitly.
Does the backend need to know about peripherals?
No
Or why do you want the LLVM backend to know when it's targeting, for example, "atmega328"?
Solely for deciding what subset of the ISA is supported.All device-specific information required (or used) by the backend can be found inside AVRDevices.td.
There is a bunch of other related discussion on https://github.com/rust-lang/rust/issues/44036 too.
Order this #include alphabetically with the ones above it