LLVM does not distinguish Cortex-M4 from Cortex-M4F neither Cortex-R5 from R5F

[labrinea]

Summary

LLVM currently treats Cortex-M4 and Cortex-R5 as having an FPU. However it distinguishes other arm processors that come optionally with an FPU. These are: cortex-r4, arm1136j-s, arm1176jz-s, mpcore, and arm1156t2-s. Apparently Target Parser and ARM.td are not synchronized.

[meadori]

lib/Target/ARM/ARM.td

470 ↗

(On Diff #34212)

Do our processor names always correspond to real processor names? It was my understanding that there is no such thing as a "Cortex-M4F" processor: http://sourceware.org/ml/binutils/2010-09/msg00184.html So, GCC & binutils definitely don't have "cortex-m4f".

FWIW, if we don't, then I think we should strive to always use actual processor names.

[rengolin]

Adding Tim as a reviewer, as he was working on M-class Darwin. I'm not sure this change is welcome, or even valid, there.

I'm not particularly bothered by having the F variants per se, but this may create a conflict when using the GNU linker (which we do), so I wouldn't want to have to fix that, given that it was already refused once by Matt.

Alexandros, any special reason why you're adding this?

cheers,
--renato

test/CodeGen/ARM/darwin-eabi.ll

4 ↗

(On Diff #34212)

Not to mention that this is changing Darwin CPU names, which are normally a lot more strict than the rest...

[labrinea]

One reason I proposed this is that tools/clang/test/CodeGen/arm-target-features.c currently tests cortex-r5f and cortex-m4f but llvm does not recognize these processor names since they are missing from lib/Target/ARM/ARM.td. The tests are passing and that is the reason I did not notice this issue before, when I was writing these tests (http://reviews.llvm.org/D11299). Another way to solve this is changing the tests and also removing the corresponding entries of cortex-r5f and cortex-m4f from Target Parser.

I am not sure what we want though, since I noticed that the processors which I listed above (cortex-r4, arm1136j-s, arm1176jz-s, mpcore, and arm1156t2-s) have "f" versions. That's why I asked for feedback about this patch in llvm-dev.

[richard.barton.arm]

Hi Alexandros

My understanding is that clang wants to match gcc for -mcpu and similar options which means supporting the inconsistency as Renato says. From checking with my 4.9.3 arm-none-eabi toolchain I think this means not supporting Cortex-M4F and Cortex-R5F, but supporting Cortex-R4F unfortunately.

Ta
Rich

[labrinea]

Previous approach was not very welcome to llvm-dev. Since we want to mimic gcc behaviour I am removing r5f and m4f entries from Target Parser. Updating r5 and m4 FPUs accordingly. A clang patch is required for these changes otherwise there will be regressions.

[rengolin]

Hi Alexandros, Richard,

It's not that we want to mimic GCC, but we also don't want to create new names that will never be used by binutils (which we still rely on).

Moreover, this change is more in sync to the LLVM philosophy that we should map the most common representation of a core, not the bare minimum. If the default configuration of R5s and M4s is to have VFP, then we shall represent it that way and let the minority of users disable them via flags.

LGTM, thanks!

[labrinea]

committed as r247136

include/llvm/Support/ARMTargetParser.def

 ARM_CPU_NAME("cortex-a15", AK_ARMV7A, FK_NEON_VFPV4, false,
              (AEK_SEC | AEK_MP | AEK_VIRT | AEK_HWDIVARM | AEK_HWDIV))
 ARM_CPU_NAME("cortex-a17", AK_ARMV7A, FK_NEON_VFPV4, false,
              (AEK_SEC | AEK_MP | AEK_VIRT | AEK_HWDIVARM | AEK_HWDIV))
 ARM_CPU_NAME("krait", AK_ARMV7A, FK_NEON_VFPV4, false,
              (AEK_HWDIVARM | AEK_HWDIV))
 ARM_CPU_NAME("cortex-r4", AK_ARMV7R, FK_NONE, true, AEK_NONE)
 ARM_CPU_NAME("cortex-r4f", AK_ARMV7R, FK_VFPV3_D16, false, AEK_NONE)
-ARM_CPU_NAME("cortex-r5", AK_ARMV7R, FK_NONE, false,
-             (AEK_MP | AEK_HWDIVARM))
-ARM_CPU_NAME("cortex-r5f", AK_ARMV7R, FK_VFPV3_D16, false,
+ARM_CPU_NAME("cortex-r5", AK_ARMV7R, FK_VFPV3_D16, false,
              (AEK_MP | AEK_HWDIVARM))
 ARM_CPU_NAME("cortex-r7", AK_ARMV7R, FK_VFPV3_D16_FP16, false,
              (AEK_MP | AEK_HWDIVARM))
 ARM_CPU_NAME("sc300", AK_ARMV7M, FK_NONE, false, AEK_NONE)
 ARM_CPU_NAME("cortex-m3", AK_ARMV7M, FK_NONE, true, AEK_NONE)
-ARM_CPU_NAME("cortex-m4", AK_ARMV7EM, FK_NONE, true, AEK_NONE)
-ARM_CPU_NAME("cortex-m4f", AK_ARMV7EM, FK_FPV4_SP_D16, true, AEK_NONE)
+ARM_CPU_NAME("cortex-m4", AK_ARMV7EM, FK_FPV4_SP_D16, true, AEK_NONE)
 ARM_CPU_NAME("cortex-m7", AK_ARMV7EM, FK_FPV5_D16, false, AEK_NONE)
 ARM_CPU_NAME("cortex-a53", AK_ARMV8A, FK_CRYPTO_NEON_FP_ARMV8, true, AEK_CRC)
 ARM_CPU_NAME("cortex-a57", AK_ARMV8A, FK_CRYPTO_NEON_FP_ARMV8, false, AEK_CRC)
 ARM_CPU_NAME("cortex-a72", AK_ARMV8A, FK_CRYPTO_NEON_FP_ARMV8, false, AEK_CRC)
 ARM_CPU_NAME("cyclone", AK_ARMV8A, FK_CRYPTO_NEON_FP_ARMV8, false, AEK_CRC)
 ARM_CPU_NAME("generic", AK_ARMV8_1A, FK_NEON_FP_ARMV8, true, AEK_NONE)
 // Non-standard Arch names.
 ARM_CPU_NAME("iwmmxt", AK_IWMMXT, FK_NONE, true, AEK_NONE)