[PATCH] D29073: [ARM] Enable Cortex-M23 and Cortex-M33 support.

[Sanne Wouda via Phabricator via llvm-commits]

sanwou01 added a comment.

In https://reviews.llvm.org/D29073#654679, @rengolin wrote:

> This patch has too many issues in one:
>
> 1. Adding new M cores to the target parser. The testing has to be done in the unit-test (as you've done). Check.
> 2. 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).


I see, I'll get on that.

> 3. Adding a test to tail call, which should belong to https://reviews.llvm.org/D29020.

Moved, sorry about that.

> 4. 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 https://reviews.llvm.org/D29020.

Thanks, will do.


https://reviews.llvm.org/D29073