This is an archive of the discontinued LLVM Phabricator instance.

[ARM] Add missing M/R class CPUs
ClosedPublic

Authored by bsmith on Feb 16 2015, 7:13 AM.

Details

Reviewers
rengolin
Summary

This patch adds some of the missing M and R class Cortex CPUs, namely:

Cortex-M0+ (called Cortex-M0plus for GCC compatibility)
Cortex-M1
SC000
SC300
Cortex-R7

Diff Detail

Repository
rL LLVM

Event Timeline

bsmith updated this revision to Diff 20029.Feb 16 2015, 7:13 AM
bsmith retitled this revision from to [ARM] Add missing M/R class CPUs.
bsmith updated this object.
bsmith edited the test plan for this revision. (Show Details)
bsmith set the repository for this revision to rL LLVM.
bsmith added a subscriber: Unknown Object (MLST).

Hi Bradley,

Do these need to be their own cores? Or could they be aliases? This will probably introduce misinformation when someone else changes "m0" but not the others for lack of information if they should...

Or maybe I'm missing the point and those cores will end up with different features in later commits.

cheers,
--renato

Some of them will end up with the same feature set (the sc* CPUs for example) so maybe aliases would be best. That said, how do you do aliases and still have them targetable in LLVM? (the only way I know of is to do it at the clang side).

Unless you mean map both CPUs to the same single subtarget feature which then maps to the correct feature set? (One issue with this is that the ARM backend is getting very close to the current 64 subtarget feature limit, I'm wary of introducing more not strictly necessary ones until this is fixed).

The problem is more of duplication in the table-gen file than the end results, which will end up as a very large table anyway.

Could you use a defm instead of def? So, for each model, you add all aliases. Ex. cortex-a8 has r5, r7, etc.

I've had a play with using defm etc for this, and whilst it does work it ends up being rather messy and unclear (especially when applied to all CPUs). I think in an ideal world the way I would structure this would be to model architectures rather than CPUs. That is, have subtarget features that represent architectures and enable the correct set of features for each, then have the CPUs inherit their architecture + extra bits rather than a full set of features. However as I mentioned, this cannot be done yet due to almost reaching the subtarget feature limit.

I think what I would propose is to commit this as is for now (perhaps with a TODO/FIXME), and once D7065 lands revisit this and restructure the whole thing to remove this kind of duplication and better represent the targets.

rengolin accepted this revision.Feb 17 2015, 7:43 AM
rengolin added a reviewer: rengolin.

Hi Bradley, I agree. LGTM, thanks!

This revision is now accepted and ready to land.Feb 17 2015, 7:43 AM
bsmith closed this revision.Feb 18 2015, 2:35 AM

Committed as 229660.