Add both cores to the target parser and TableGen. Tests that eabi
attributes are set correctly for both cores. Additionally, test the
absence and presence of MOVT in Cortex-M23 and Cortex-M33, respectively.
Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
Depends on https://reviews.llvm.org/D29020 for cortex-m23.ll to pass. FYI, I do not have commit access.
This patch has too many issues in one:
- Adding new M cores to the target parser. The testing has to be done in the unit-test (as you've done). Check.
- Adding new M cores to TableGen. Testing needs to be done by adding the new core to existing tests and using the appropriate CHECK lines (not adding new ones).
- Adding a test to tail call, which should belong to D29020.
- Heavily modifying the SW/HWDIV test, even though the hardware description you added in TableGen has no mention of special DIV features that would separate it from the other M cores.
I suggest you:
A. Keep the Target Parser and the TableGen changes here, but instead of modifying existing tests, just add new RUN lines to them where other M cores are tested and reuse the existing CHECK lines.
B. Move the tail-call test to D29020.
cheers,
--renato
test/CodeGen/ARM/cortex-m23.ll | ||
---|---|---|
1 ↗ | (On Diff #85539) | This test is completely different to the patch, I'm not sure why you moved it here. |
test/CodeGen/ARM/div.ll | ||
2 ↗ | (On Diff #85539) | Too many check lines increase the risk of tests being ignored, especially when you just rename them below, instead of adding new ones. |
I see, I'll get on that.
- Adding a test to tail call, which should belong to D29020.
Moved, sorry about that.
- Heavily modifying the SW/HWDIV test, even though the hardware description you added in TableGen has no mention of special DIV features that would separate it from the other M cores.
The cortex-m23 is sufficiently different from the other processors checked in this file that the extra CHECK lines were necessary. I can add a new file to test HWDIV on Thumb 1. I'll submit a separate patch for that.
I suggest you:
A. Keep the Target Parser and the TableGen changes here, but instead of modifying existing tests, just add new RUN lines to them where other M cores are tested and reuse the existing CHECK lines.
B. Move the tail-call test to D29020.
Thanks, will do.
Right, extra checks are ok, changing the old check is risky. If there's too much bloat, you can split the test or just duplicate some of the CHECK lines.
The Thumb1 HWDIV test is probably best on its own.
cheers,
--renato