peter.smith added a comment. I've made a suggestion to disallow v8-m.baseline (does not have Thumb 2 but has number > 7) and to simplify the expression.
================ Comment at: clang/lib/Driver/ToolChains/Arch/ARM.cpp:155 + llvm::ARM::ArchKind AK = llvm::ARM::parseArch(Triple.getArchName()); + return Ver >= 7 || AK == llvm::ARM::ArchKind::ARMV6T2 || + (Ver == 6 && Triple.isARM()); ---------------- nickdesaulniers wrote: > peter.smith wrote: > > nickdesaulniers wrote: > > > ardb wrote: > > > > peter.smith wrote: > > > > > Are we restricting based on whether the threadid register is present > > > > > or whether the instructions are available to access the cp15 > > > > > registers? > > > > > > > > > > If we're going by whether the CPU has the register present then it > > > > > will be > > > > > * A and R profile (not M profile, even the ones that have Thumb2) > > > > > * V6K (includes ARM1176 but not ARM1156t2-s which has Thumb-2!) and > > > > > V7+ (A and R profile) > > > > > > > > > > If we're going by the instructions to write to CP15 then it is: > > > > > * Arm state (everything) > > > > > * Thumb2 (v7 + v6t2) > > > > > > > > > > The above seems to be a blend of the two. Is it worth choosing one > > > > > form or the other? GCC seems to use the latter. I guess using this > > > > > option falls into the I know what I'm doing area that accessing named > > > > > system registers comes into. If the kernel supports it the stricter > > > > > version may help catch more mistakes though. > > > > > > > > > > The v7 A/R Arm ARM https://developer.arm.com/documentation/ddi0403/ed > > > > > has in `D12.7.21 CP15 c13, Context ID support` > > > > > ``` An ARMv6K implementation requires the Software Thread ID > > > > > registers described in VMSA CP15 c13 > > > > > register summary, Process, context and thread ID registers on page > > > > > B3-1474. ``` > > > > > > > > > > The Arm 1156-s (the only v6t2 processor) TRM > > > > > https://developer.arm.com/documentation/ddi0338/g/system-control-coprocessor/system-control-processor-registers/c13--process-id-register?lang=en > > > > > which shows only one process ID register under opcode 1 accessed via: > > > > > ``` > > > > > MRC p15, 0, <Rd>, c13, c0, 1 ;Read Process ID Register > > > > > ``` > > > > > Whereas the ThreadID register is opcode 3 on CPUs that are v6k and v7. > > > > The primary reason for tightening these checks was to avoid an assert > > > > in the backend when using -mtp=cp15 with a Thumb1 target, so I'd say > > > > this is more about whether the ISA has the opcode to begin with, rather > > > > than whether CPU x implements it or not. > > > > Arm state (everything) > > > > > > > > > Does that mean that `-march=armv5 -marm` has access/support for "CP15 C13 > > > ThreadID" access? > > The co-processor instructions are the same, but the effect of the > > instructions will depend on the CPU. For example on armv5 we can write the > > instruction to read/write to CP15 C13 with the ThreadID opcode. However on > > no armv5 implementation will the CP15 C13 have a Thread ID register. > > > > The more complex stricter check will make sure that the implementations > > have the ThreadID register. As Ard mentions, the GCC intent seems to be > > whether the instruction is encodable rather than check what the CPU > > supports. > I think this thread is resolved, so marking as done. Please reopen it though > if I misunderstood. I think the discussion about whether the MRC/MCR instructions are available is closed. I think the expression isn't quite right as > 7 will permit v8-m.baseline, which is the evolution of cortex-m0. To match GCC I think you want the equivalent of ARM || hasThumb2Instructions() I think you can do that with the equivalent of ArmTargetInfo::supportsThumb2() from lib/Basic/Targets/ARM.cpp ``` bool ARMTargetInfo::supportsThumb2() const { return CPUAttr.equals("6T2") || (ArchVersion >= 7 && !CPUAttr.equals("8M_BASE")); } ``` Where CPUAttr.equals("8M_BASE") is equivalent to `llvm::ARM::ArchKind::ARMV8MBaseline` so I think you can use ``` return Triple.isARM() || (Ver >= 7 && AK != llvm::ARM::ArchKind::ARMV8MBaseline); ``` ARMV6K and ARMV6KZ can only MRC and MCR in ARM mode so are covered by Triple.isARM(). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114116/new/ https://reviews.llvm.org/D114116 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits