Page MenuHomePhabricator

[clang][ARM] Remove arm2/3/6/7m CPU names
ClosedPublic

Authored by DavidSpickett on May 24 2021, 7:40 AM.

Details

Summary

These legacy CPUs are known to clang but not llvm.
Their use was ignored by llvm and it would print a
warning saying it did not recognise them.

However because some of them are default CPUs for their
architecture, you would get those warnings even if you didn't
choose a cpu explicitly.
(now those architectures will default to a "generic" CPU)

Information is thin on the ground for these older chips
so this is the best I could find:
https://en.wikichip.org/wiki/acorn/microarchitectures/arm2
https://en.wikichip.org/wiki/acorn/microarchitectures/arm3
https://en.wikichip.org/wiki/arm_holdings/microarchitectures/arm6
https://en.wikichip.org/wiki/arm_holdings/microarchitectures/arm7

Final part of fixing https://bugs.llvm.org/show_bug.cgi?id=50454.

Diff Detail

Event Timeline

DavidSpickett created this revision.May 24 2021, 7:40 AM
DavidSpickett requested review of this revision.May 24 2021, 7:40 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 24 2021, 7:40 AM

Does LLVM generate correct code for those architectures? For example armv3 doesn't have BX LR, it must use MOV PC, LR or emit BX LR with an R_ARM_V4BX relocation so the linker can transform it. From memory I think armv2 doesn't even use 32-bit addresses, the flags were stored in the top 8 bits.

The ARM documentation people usually use doesn't document anything older than v4, and I've never seen anyone use those targets. I'd rather just reject them from clang.

DavidSpickett planned changes to this revision.May 25 2021, 1:18 AM

@nickdesaulniers Can you confirm whether the kernel CI report you got was intentionally setting armv3m? If the build isn't setting an mcpu then removing the CPUs won't actually break it, though what it generates likely wouldn't run correctly.

In the meantime I will have a scan through the code and see if this armv4 minimum is evident in llvm. Then update this change to remove the CPUs from clang.

@nickdesaulniers Can you confirm whether the kernel CI report you got was intentionally setting armv3m? If the build isn't setting an mcpu then removing the CPUs won't actually break it, though what it generates likely wouldn't run correctly.

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/Makefile#n74 explicitly sets -march=armv3m, when trying to build CONFIG_CPU_32v3. We don't yet support v4 or v4t FWIW.

I found some discussion at https://patchwork.kernel.org/project/linux-arm-kernel/patch/20180930024904.14240-1-Jason@zx2c4.com/ . Apparently, the kernel is actually intentionally building for "ARMv3M" by default, to support some ancient hardware that doesn't support all the armv4 instructions. LLVM doesn't support this at the moment. We shouldn't accept -march=armv3m unless we're actually going to generate correct code for it.

We don't yet support v4 or v4t FWIW.

Not sure what you mean by this?

I found some discussion at https://patchwork.kernel.org/project/linux-arm-kernel/patch/20180930024904.14240-1-Jason@zx2c4.com/ . Apparently, the kernel is actually intentionally building for "ARMv3M" by default,

(when targeting armv3 hardware, not in general)

to support some ancient hardware that doesn't support all the armv4 instructions. LLVM doesn't support this at the moment. We shouldn't accept -march=armv3m unless we're actually going to generate correct code for it.

We don't yet support v4 or v4t FWIW.

Not sure what you mean by this?

Folks building the Linux kernel with Clang do not have working configurations of the Linux kernel that target ARMv4 hardware. I don't know the list of blockers, and it's not a high priority. Clang may support ARMv4, but that doesn't mean a kernel with such configuration with build let alone boot.

Instead of adding CPUs to llvm, remove them from clang.

If anyone was using them they weren't making any difference,
apart from generating warnings.

I checked the linux tree and found only 2 similair strings.
A Freescale "Armardillo2" aka arm2 board, which is actually using
a cortex A-9, and a comment in a file for the ARM720 which
is actually an ARM7TDMI-S CPU.

Whether we generate correct code for pre v4 is a
question for a different time, so the architectures remain.

Herald added a project: Restricted Project. · View Herald TranscriptJun 2 2021, 3:36 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
DavidSpickett retitled this revision from [llvm][ARM] Add CPU defs for arm2/3/6/7m to [clang][ARM] Remove arm2/3/6/7m CPU names.Jun 2 2021, 3:37 AM
DavidSpickett edited the summary of this revision. (Show Details)
DavidSpickett added inline comments.
clang/test/Misc/target-invalid-cpu-note.c
4

Despite the triple being armv5, clang just prints every CPU it knows about. The arm8 is now the first one.

This revision is now accepted and ready to land.Jun 2 2021, 11:14 AM

Clang format

This revision was landed with ongoing or failed builds.Jun 3 2021, 1:55 AM
This revision was automatically updated to reflect the committed changes.