[ARM] Disallow PC, and optionally SP, in VMOVRH and VMOVHR.

[simon_tatham]

Summary

Arm v8.1-M supports the VMOV instructions that move a half-precision
value to and from a GPR, but not if the GPR is SP or PC.

To fix this, I've changed those instructions to use the rGPR register
class instead of GPR. rGPR always excludes PC, and it excludes SP
except in the presence of the HasV8Ops target feature (i.e. Arm v8-A).
So the effect is that VMOV.F16 to and from PC is now illegal
everywhere, but VMOV.F16 to and from SP is illegal only on non-v8-A
cores (which I believe is all as it should be).

[dmgreen]

I agree with this. Does it have a test anywhere?

[simon_tatham]

It's tested for v8.1-M, by llvm/test/MC/ARM/thumbv8.1m.s which is added by D60706. I could add an extra test or two to this patch for other affected architectures, if you think it's necessary.

[ostannard]

I think it would be worth a test in this patch, especially since D60706 only tests v8.1M, not v8.2A which also introduces this instruction.

[simon_tatham]

OK, here's a revised version which adds a test for v8.2A.

[ostannard]

llvm/test/MC/ARM/vmovhr.s

1 ↗

(On Diff #202220)

The rGPR register class should already handle the difference in use of SP from v8 onwards. I think the nonsense "thumbv8.2a.main" triple here is preventing that from working.

[simon_tatham]

Fixed combination of .main with v8.2a, and updated expected test
results accordingly. The commit message wants a rewrite as well, which
I'll upload in a moment.

[ostannard]

LGTM

llvm/trunk/lib/Target/ARM/ARMInstrVFP.td

def VMOVSRR : AVConv5I<0b11000100, 0b1010,
   // pipelines.
   let D = VFPNeonDomain;
 
   let DecoderMethod = "DecodeVMOVSRR";
 }
 
 // Move H->R, clearing top 16 bits
 def VMOVRH : AVConv2I<0b11100001, 0b1001,
-                      (outs GPR:$Rt), (ins HPR:$Sn),
+                      (outs rGPR:$Rt), (ins HPR:$Sn),
                       IIC_fpMOVSI, "vmov", ".f16\t$Rt, $Sn",
-                      [(set GPR:$Rt, (arm_vmovrh HPR:$Sn))]>,
+                      [(set rGPR:$Rt, (arm_vmovrh HPR:$Sn))]>,
              Requires<[HasFPRegs16]>,
              Sched<[WriteFPMOV]> {
   // Instruction operands.
   bits<4> Rt;
   bits<5> Sn;
 
   // Encode instruction operands.
   let Inst{19-16} = Sn{4-1};
   let Inst{7}     = Sn{0};
   let Inst{15-12} = Rt;
 
   let Inst{6-5}   = 0b00;
   let Inst{3-0}   = 0b0000;
 
   let isUnpredicable = 1;
 }
 
 // Move R->H, clearing top 16 bits
 def VMOVHR : AVConv4I<0b11100000, 0b1001,
-                      (outs HPR:$Sn), (ins GPR:$Rt),
+                      (outs HPR:$Sn), (ins rGPR:$Rt),
                       IIC_fpMOVIS, "vmov", ".f16\t$Sn, $Rt",
-                      [(set HPR:$Sn, (arm_vmovhr GPR:$Rt))]>,
+                      [(set HPR:$Sn, (arm_vmovhr rGPR:$Rt))]>,
              Requires<[HasFPRegs16]>,
              Sched<[WriteFPMOV]> {
   // Instruction operands.
   bits<5> Sn;
   bits<4> Rt;
 
   // Encode instruction operands.
   let Inst{19-16} = Sn{4-1};

llvm/trunk/test/MC/ARM/vmovhr.s

+// RUN: not llvm-mc -triple=thumbv8.2a-none-eabi -mattr=+fp-armv8,+fullfp16 -show-encoding < %s 2>%t \
+// RUN:   | FileCheck %s
+// RUN:     FileCheck --check-prefix=ERROR < %t %s
+
+# CHECK: vmov.f16 r0, s13 @ encoding: [0x16,0xee,0x90,0x09]
+vmov.f16 r0, s13
+
+# CHECK: vmov.f16 s21, r1 @ encoding: [0x0a,0xee,0x90,0x19]
+vmov.f16 s21, r1
+
+# CHECK: vmov.f16 s2, sp @ encoding: [0x01,0xee,0x10,0xd9]
+vmov.f16 s2, sp
+
+# ERROR: :[[@LINE+2]]:{{[0-9]+}}: error: invalid instruction
+# ERROR: operand must be a register in range [r0, r14]
+vmov.f16 s3, pc
+
+# CHECK: vmov.f16 sp, s5 @ encoding: [0x12,0xee,0x90,0xd9]
+vmov.f16 sp, s5
+
+# ERROR: :[[@LINE+2]]:{{[0-9]+}}: error: invalid instruction
+# ERROR: operand must be a register in range [r0, r14]
+vmov.f16 pc, s8
+