[ARM] Removing v7s/v7f/v7k and cortex-a9-mp

Hi,

I'd like to propose making a change in the ARM backend such that the MP
extensions subtarget feature is enabled by default on all CPUs with this
extension, however this has an impact on various other areas of Clang and
LLVM.

Most notably, Clang has a CPU name, cortex-a9-mp, that should be gotten rid
of as part of this change since it will be redundant in terms of subtarget
features enabled, (it would be identical to cortex-a9). However currently,
this CPU name maps to an architecture of armv7f. This looks to be a pseudo
architecture that from what I can gather is only used for the CPUSubType
field in the Mach-O header.

In order to avoid keeping this cortex-a9-mp CPU around (since it will just
be a source of confusion), I would also like to propose removing these
Mach-O specific architectures from Clang (7f, 7s, 7k) and instead pass this
information through to LLVM in a different manner, perhaps by changing the
CPUSubType logic in the ARM backend to be based upon CPU rather than these
pseudo architectures, or by passing it by command line? (I'm guessing care
will need to be taken to avoid letting CPUs that don't map to a valid
CPUSubType get through?).

Both of these changes should make the Clang driver a bit more sane in these
areas, so I think these are both generally good changes to do. Does anybody
have any comments/objections to this? Specifically, am I right in thinking
that the Mach-O header is the only place these pseudo architectures are
being used? What would be the right approach for getting the CPUSubType
information through to LLVM?

Regards,
Bradley Smith

Can you clarify what you are proposing? Are you proposing that we eliminate the option such as -arch armv7s? If so, the answer is no. This would impact a lot of users negatively.

If you are proposing changes to how the information is communicated between the frontend and backend, then the answer is maybe. You do need to provide a much more concrete proposal that will maintain the functionality.

Evan

Can you clarify what you are proposing? Are you proposing that we
eliminate the option such as -arch armv7s? If so, the answer is no. This
would impact a lot of users negatively.

I agree this is not a good idea, nor it should be required on the clang or
llvm drivers to spot and A core with MP enabled.

If you are proposing changes to how the information is communicated between

the frontend and backend, then the answer is maybe. You do need to provide
a much more concrete proposal that will maintain the functionality.

I thought a bit about this a while ago, and my impression was that adding a
generic flag -mattr=+mp would do the trick between Clang and LLVM, changing
the "a9-mp" CPU name to only a9 and adding the new flag. It should be
simple also to map "-mcpu cortex-a9-mp" on the command line to "armv7s" +
"cortex-a9" + "-mattr=+mp" options.

I can't see how that could impact Darwin in the long term, so I defer to
Evan to make sure it won't. :wink:

cheers,
--renato

Hi Evan,

I think this may have come across as more of a proposal than I intended, I
really want to gather information such that I can propose an appropriate
patch on list. (As it stood the patch I would have proposed would have
simply removed 7s/7k/7f since I did not know others requirements properly,
and I felt that creating such a patch would be wasted effort since I
expected such a patch to get rejected).

I'll step back from what I wrote previously and try to explain exactly what
I'm trying to achieve.

Currently the LLVM backend has the following CPU -> feature mappings:

cortex-a9 -> A9
cortex-a9-mp -> A9 + MP
swift -> swift + MP
cortex-a15 -> A15 + MP
cortex-a53 -> A53 + MP
cortex-a57 -> A57 + MP

A9 is the odd one out here in that it enables MP via a separate CPU name
rather than by default, my aim is to make this consistent across LLVM/Clang.
The sensible way to address this it seems is to remove cortex-a9-mp
altogether and enable MP by default on cortex-a9 (and also add MP to all
other relevant cores as a default feature).

* Do you have any expectation that users will need the CPU name cortex-a9-mp
in LLVM?

If not, the next step is to make the frontend consistent with this, that is,
remove cortex-a9-mp and inherit the fact that cortex-a9 provides MP.

* Similarly, do you expect users to need cortex-a9-mp in Clang?

Even if not, this presents a problem, Clang has the following CPU ->
architecture mappings:

cortex-a9 -> armv7a
cortex-a9-mp -> armv7f

If we were to simply remove cortex-a9-mp then there would no longer be any
CPU that mapped to the armv7f architecture, which I assume would be a
problem for users? The naive answer is to remove v7f and pass the CPUSubType
information it provides to the backend in a different manner, however as you
have stated this is not an option due to needing to keep -arch armv7f etc.

Another approach is to not have these architectures as real architectures in
the LLVM sense, and instead introduce a CPUSubType option (or something like
this) to LLVM. The CPU cortex-a9 could then always map to v7a and in some
separate logic Clang could figure out a CPUSubType in addition. That way we
get the benefits of the naive approach, yet preserve the -arch options. This
could result in something like the following mappings:

-march=v7f -> -march=v7a + v7f-CPUSubType
-mcpu=cortex-a9 -> -mcpu=cortex-a9 + v7f-CPUSubType (-march=v7a).

This could also be OS/driver agnostic, since the backend can simply ignore
the CPUSubType if it doesn't care, or instead clang could only generate and
pass it for a relevant OS. However this approach would remove v7f as an
architecture from LLVM, would this be an issue?

Are the v7f/v7k/v7s architectures only relevant for Darwin/iOS targets? If
so, another approach could be to move these architectures such that they are
only accepted for such targets? In which case, cortex-a9 would map to v7f
for Darwin/iOS and would map to v7a elsewhere. Would such a change be too
confusing for users?

If none of these approaches work could you explain what your requirements
are for these architectures (and cortex-a9-mp if relevant), so that I can
come up with a solution that might work? Thanks.

Regards,
Bradley Smith

Ok, if your main concern is to get rid of the cortex-a9-mp cpu, then I would follow Renato's suggestion of just adding a -mattr=+mp option. I don't see a bigger change like you suggested below is worth it.

Evan

Ok, if your main concern is to get rid of the cortex-a9-mp cpu, then I would follow Renato's suggestion of just adding a -mattr=+mp option. I don't see a bigger change like you suggested below is worth it.

Evan

That would work, but it would be good to clean up this stuff a bit and I think Bradley is on the right track.

First off, the issue here is specifically v7f. It has nothing to do with either v7s or v7k. I will need to double check, but I’m not aware of any fundamental reason why we need to keep v7f around. If we remove v7f, then I’m not aware of any reason to keep the cortex-a9-mp cpu.

> Ok, if your main concern is to get rid of the cortex-a9-mp cpu, then
I would follow Renato's suggestion of just adding a -mattr=+mp option.
I don't see a bigger change like you suggested below is worth it.
>
> Evan

That would work, but it would be good to clean up this stuff a bit and
I think Bradley is on the right track.

First off, the issue here is specifically v7f. It has nothing to do
with either v7s or v7k. I will need to double check, but I'm not aware
of any fundamental reason why we need to keep v7f around. If we remove
v7f, then I'm not aware of any reason to keep the cortex-a9-mp cpu.

Our immediate concern is indeed just v7f, consistency says that whatever we
do to v7f we do to the others, however that depends entirely on others
requirements, we don't have a requirement to do anything to v7s and v7k so
can ignore those if needed.

For v7f, are you suggesting that we take the approach of adding a CPUSubType
option to pass information through to LLVM? Or that we may be able to get
rid of it entirely and forget about CPUSubType generation for v7f
specifically?

Regards,
Bradley Smith

I am suggesting that we completely remove the armv7f architecture along with the cortex-a9-mp cpu. I need to do some checking first to make sure no one is using that. It will take a little while to do that checking, but I think that would be the right solution. I will get back to you.

As expected, it took a while, but I have now removed armv7f. I kept the code to recognize the cortex-a9-mp cpu name, but changed to match to the base armv7 architecture.

clang svn r199336
llvm svn r199337
compiler-rt svn r199333

Hopefully that will unblock the other changes that you wanted to make.