[AArch64]: Add support for parsing rN registers.

[manojgupta]

Summary

Allow rN registers to be simply parsed as correspoing xN registers.
The "register ... asm("rN")" is an command to the
compiler's register allocator, not an operand to any individual assembly
instruction. GCC documents this syntax as "...the name of the register
that should be used."

This is needed to support the changes in Linux kernel (see
https://lkml.org/lkml/2018/3/1/268 )

Note: This will add support only for the limited use case of
register ... asm("rN"). Any other uses that make rN leak into assembly
are not supported.

[manojgupta]

Indenting and a minor fix.

[manojgupta]

Add more context from the email communication with Peter Smith and Robin Murphy.

On 22/03/18 02:34, Manoj Gupta wrote:

Hi Robin,

this is Manoj from Google. I was looking into implementing support for
parsing "r" registers in clang for AArch64 based on lkml thread
https://lkml.org/lkml/2018/3/1/186 ).

On the llvm bug(https://bugs.llvm.org/show_bug.cgi?id=36862#c2 ), Peter had
a comment that I think could be better clarified by you.

"
Can you be a bit more specific about where in GCC "rn" is acceptable? It is
certainly not true in the general case, for example:
...
"
Further, my understanding is that binutils does not support "r" registers
in AArch64.
So is it correct that GCC support limited to parsing "r" registers as "x"
registers and assembler will never see "r" registers?

There are two distinct things being conflated here: "r0" is not, and
never has been, a valid A64 assembly *operand*, thus indeed should never
be passed to AArch64 binutils since the latter only consumes A64
assembly syntax.

The "register ... asm("r0")" case, however, is an command to the
compiler's register allocator, not an operand to any individual assembly
instruction. GCC documents this syntax as "...the name of the register
that should be used.", and section B1.2.1 of the Armv8 ARM[1] is quite
clear that the fundamental AArch64 registers are named R0-R30, SP, PC,
and V0-V31. It seems perfectly straightforward to me that the register
allocator would work in units of fundamental machine registers, while
access/element sizes only start to matter in the subsequent assembly
generation phase where actual instruction accesses to those registers
are emitted (and indeed where those sizes depend more on each specific
operation than the inherent size of the type being operated on).

I note that Clang *does* already understand references to SIMD&FP "V"
registers in this context as expected[2] without demanding an explicit
access or element size.

Robin.

[1]
https://developer.arm.com/products/architecture/a-profile/docs/ddi0487/latest/arm-architecture-reference-manual-armv8-for-armv8-a-architecture-profile
[2]
http://lists.infradead.org/pipermail/linux-arm-kernel/2018-March/567872.html

[srhines]

Peter also requested that a test be added to make sure that rY was not accepted by the Clang assembler as a true synonym for xY.

lib/Basic/Targets/AArch64.cpp

320 ↗

(On Diff #139556)

For x29, x30, you should really be grouping them together. For instance:

{{"r29", "fp"}, "x29}, {{"r30", "lr"}, "x30"}

This lets you remove the x29/fp and x30/lr from the first line.

[manojgupta]

In D44815#1046451, @srhines wrote:

Peter also requested that a test be added to make sure that rY was not accepted by the Clang assembler as a true synonym for xY.

Yes, I am working on that. Wanted to send this out first.

[manojgupta]

1. Added test case to verify register names are not renamed in unsupported cases.
2. Updated formatting.

[peter.smith]

I think that we may have a bit of a conceptual difference with GCC here. As far as I can tell in GCC it doesn't matter if "rn", "wn", "xn" or even "bn" is used, this refers to register 0. The Operand constraint such as %w0 is used to control the substitution so the r registers aren't strictly aliases like the other registers. The comment

The S/D/Q and W/X registers overlap, but aren't really aliases; we
don't want to substitute one of these for a different-sized one.

suggests that this may have been the intent that Clang behave differently to GCC in this respect. For example in Clang there appears to be a bit more significance placed on the "wn" or "xn", for example clang will warn in the following example, whereas gcc will not:

long f2(int i) {
    register int x asm("w6");
    register int y asm("w7");
    register int z asm("w8");
    asm("add %0, %1, %2\n": "=r"(x) : "r"(y), "r"(z));
    return y;
}

t2.c:5:34: warning: value size does not match register size specified by the
      constraint and modifier [-Wasm-operand-widths]
    asm("add %0, %1, %2\n": "=r"(x) : "r"(y), "r"(z));
                     ^~
                     %w0
// and so on for %1 and %2.

Having said all that, clang will not warn when the operand modifier is explicit, for example:

long f2(int i) {
    register long x asm("w6");
    register long y asm("w7");
    register long z asm("w8");
    asm("add %w0, %w1, %w2\n": "=r"(x) : "r"(y), "r"(z));
    return y;
}

At the moment I'm not too sure what to make of this. Strictly speaking I think "rn" is an unsized register that Clang shouldn't warn about size mismatch in the operand modifier. With the implementation above it won't because we default to xn and clang doesn't seem to warn when there is an explicit modifier for wn, although this could change in the future.

I'd be very interested to hear other opinions on this? If register rn is to be supported perhaps a test that clang doesn't warn when asm("rn") is used with the operand modifier %wn is used.

[t.p.northover]

The warning you're seeing is because without an operand modifier Clang always chooses an x-register in its textual assembly expansion. This 64-bit default is compared to the underlying C type to determine whether a warning should be emitted, and I think that's reasonable. The C type is all that's available for most uses (without a register keyword) and it seems consistent to use it even when register has been specified on the variable.

Where we might want to introduce a new warning is for a mismatch like:

register uint64_t var asm("w0"); // 64-bits in a 32-bit register. Magic!

But that seems pretty separate to this patch really.

But

[peter.smith]

Thanks for the clarification. With that in mind I'm much less concerned that adding "r" as an alias will make extra warnings more difficult. I agree that there should be at least a warning for register long x asm ("wn") although that is separate from this patch. Has anyone else got any objections?

[peter.smith]

Given that this is important for compiling the Linux kernel with clang I think that it is worth improving compatibility with GCC. LGTM.

cfe/trunk/lib/Basic/Targets/AArch64.cpp

const char *const AArch64TargetInfo::GCCRegNames[] = {
     "v23", "v24", "v25", "v26", "v27", "v28", "v29", "v30", "v31"
 };
 
 ArrayRef<const char *> AArch64TargetInfo::getGCCRegNames() const {
   return llvm::makeArrayRef(GCCRegNames);
 }
 
 const TargetInfo::GCCRegAlias AArch64TargetInfo::GCCRegAliases[] = {
-    {{"w31"}, "wsp"}, {{"x29"}, "fp"}, {{"x30"}, "lr"}, {{"x31"}, "sp"},
+    {{"w31"}, "wsp"},
+    {{"x31"}, "sp"},
+    // GCC rN registers are aliases of xN registers.
+    {{"r0"}, "x0"},
+    {{"r1"}, "x1"},
+    {{"r2"}, "x2"},
+    {{"r3"}, "x3"},
+    {{"r4"}, "x4"},
+    {{"r5"}, "x5"},
+    {{"r6"}, "x6"},
+    {{"r7"}, "x7"},
+    {{"r8"}, "x8"},
+    {{"r9"}, "x9"},
+    {{"r10"}, "x10"},
+    {{"r11"}, "x11"},
+    {{"r12"}, "x12"},
+    {{"r13"}, "x13"},
+    {{"r14"}, "x14"},
+    {{"r15"}, "x15"},
+    {{"r16"}, "x16"},
+    {{"r17"}, "x17"},
+    {{"r18"}, "x18"},
+    {{"r19"}, "x19"},
+    {{"r20"}, "x20"},
+    {{"r21"}, "x21"},
+    {{"r22"}, "x22"},
+    {{"r23"}, "x23"},
+    {{"r24"}, "x24"},
+    {{"r25"}, "x25"},
+    {{"r26"}, "x26"},
+    {{"r27"}, "x27"},
+    {{"r28"}, "x28"},
+    {{"r29", "x29"}, "fp"},
+    {{"r30", "x30"}, "lr"},
     // The S/D/Q and W/X registers overlap, but aren't really aliases; we
     // don't want to substitute one of these for a different-sized one.
 };
 
 ArrayRef<TargetInfo::GCCRegAlias> AArch64TargetInfo::getGCCRegAliases() const {
   return llvm::makeArrayRef(GCCRegAliases);
 }
 

cfe/trunk/test/CodeGen/aarch64-inline-asm.c

 // CHECK: call i32* asm "adrp $0, ${1:A}\0A\09add $0, $0, ${1:L}", "=r,S"(i64* @var)
 }
 
 void test_constraint_Q(void) {
     int val;
     asm("ldxr %0, %1" : "=r"(val) : "Q"(var));
 // CHECK: call i32 asm "ldxr $0, $1", "=r,*Q"(i64* @var)
 }
+
+void test_gcc_registers(void) {
+    register unsigned long reg0 asm("r0") = 0;
+    register unsigned long reg1 asm("r1") = 1;
+    register unsigned int  reg29 asm("r29") = 2;
+    register unsigned int  reg30 asm("r30") = 3;
+
+    // Test remapping register names in register ... asm("rN") statments.
+    // rN register operands in these two inline assembly lines
+    // should get renamed to valid AArch64 registers.
+    asm volatile("hvc #0" : : "r" (reg0), "r" (reg1));
+    // CHECK: call void asm sideeffect "hvc #0", "{x0},{x1}"
+    asm volatile("hvc #0" : : "r" (reg29), "r" (reg30));
+    // CHECK: call void asm sideeffect "hvc #0", "{fp},{lr}"
+
+    // rN registers when used without register ... asm("rN") syntax
+    // should not be remapped.
+    asm volatile("mov r0, r1\n");
+    // CHECK: call void asm sideeffect "mov r0, r1\0A", ""()
+}