peter.smith added a comment.
Apologies, missed a couple of small things out. Otherwise looks good to me.
================
Comment at: clang/lib/Driver/ToolChains/Arch/ARM.cpp:150
+// The backend does not have support for hard thread pointers when targeting
+// Thumb1.
----------------
peter.smith wrote:
> Will be worth saying `CP15 C13 ThreadID` somewhere to make it easier to look
> up in the documentation.
Now we're permitting anything with the co-processor instructions I think the
comment would be better expressed as:
``` We follow GCC and support when the backend has support for the MRC/MCR
instructions that are used to set the hard thread pointer ("CP15 C13 //Thread
id").```
================
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());
----------------
peter.smith wrote:
> 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().
>
>
My apologies I missed out V6T2 in my comment above. I've added it back in below.
```
return Triple.isARM() || AK == llvm::ARM::ArchKind::ARMV6T2 || (Ver >= 7 && AK
!= llvm::ARM::ArchKind::ARMV8MBaseline);
```
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
