[ARM] Add vrev32 NEON fp16 patterns

[dmgreen]

Summary

These Neon patterns for vrev32.16 appear to be missing, only the i16 patterns are present.

[SjoerdMeijer]

These Neon patterns for vrev32.16 appear to be missing, only the i16 patterns are present.

Hmm, that's kind of what I was expecting: support for i16s, but nog f16s because that not only requires HasNeon but also HasFullFP16. Without HasFullFP16, are vectors of f16s legal?

[dmgreen]

Yep! That's what I meant. Neon+fullfp16, this fails to select. This test runs in that configuration now. The RHS check lines are missing because it crashes failing to select.

Without fullfp16 you are right that we would never see a f16 vector, so the patterns would never be used. We would promote the f16's to f32's before that point, I believe.

[SjoerdMeijer]

In D69508#1723357, @dmgreen wrote:

Yep! That's what I meant. Neon+fullfp16, this fails to select. This test runs in that configuration now. The RHS check lines are missing because it crashes failing to select.

Without fullfp16 you are right that we would never see a f16 vector, so the patterns would never be used. We would promote the f16's to f32's before that point, I believe.

Okidoki, cheers, that makes perfect sense.
Does that mean we only need to add HasFullFP16 to the Predicates of the patterns?

[dmgreen]

I think not, because the instruction it is actually selecting is just a vrev32, available whenever Neon is present. They will just never be used when fullfp16 is not present. Same as the existing patterns on line 6800 (which I just formatted here).

[SjoerdMeijer]

Yep, cheers, LGTM

llvm/lib/Target/ARM/ARMInstrNEON.td

 def : Pat<(v2f32 (ARMvrev64 (v2f32 DPR:$Vm))), (VREV64d32 DPR:$Vm)>;
 }
 
 def VREV64q8  : VREV64Q<0b00, "vrev64", "8", v16i8>;
 def VREV64q16 : VREV64Q<0b01, "vrev64", "16", v8i16>;
 def VREV64q32 : VREV64Q<0b10, "vrev64", "32", v4i32>;
 
 let Predicates = [HasNEON] in {
-def : Pat<(v4f32 (ARMvrev64 (v4f32 QPR:$Vm))), (VREV64q32 QPR:$Vm)>;
-def : Pat<(v8f16 (ARMvrev64 (v8f16 QPR:$Vm))), (VREV64q16 QPR:$Vm)>;
-def : Pat<(v4f16 (ARMvrev64 (v4f16 DPR:$Vm))), (VREV64d16 DPR:$Vm)>;
+  def : Pat<(v4f32 (ARMvrev64 (v4f32 QPR:$Vm))),
+            (VREV64q32 QPR:$Vm)>;
+  def : Pat<(v8f16 (ARMvrev64 (v8f16 QPR:$Vm))),
+            (VREV64q16 QPR:$Vm)>;
+  def : Pat<(v4f16 (ARMvrev64 (v4f16 DPR:$Vm))),
+            (VREV64d16 DPR:$Vm)>;
 }
 
 //   VREV32   : Vector Reverse elements within 32-bit words
 
 class VREV32D<bits<2> op19_18, string OpcodeStr, string Dt, ValueType Ty>
   : N2V<0b11, 0b11, op19_18, 0b00, 0b00001, 0, 0, (outs DPR:$Vd),
         (ins DPR:$Vm), IIC_VMOVD,
         OpcodeStr, Dt, "$Vd, $Vm", "",
         [(set DPR:$Vd, (Ty (ARMvrev32 (Ty DPR:$Vm))))]>;
 class VREV32Q<bits<2> op19_18, string OpcodeStr, string Dt, ValueType Ty>
   : N2V<0b11, 0b11, op19_18, 0b00, 0b00001, 1, 0, (outs QPR:$Vd),
         (ins QPR:$Vm), IIC_VMOVQ,
         OpcodeStr, Dt, "$Vd, $Vm", "",
         [(set QPR:$Vd, (Ty (ARMvrev32 (Ty QPR:$Vm))))]>;
 
 def VREV32d8  : VREV32D<0b00, "vrev32", "8", v8i8>;
 def VREV32d16 : VREV32D<0b01, "vrev32", "16", v4i16>;
 
 def VREV32q8  : VREV32Q<0b00, "vrev32", "8", v16i8>;
 def VREV32q16 : VREV32Q<0b01, "vrev32", "16", v8i16>;
 
+let Predicates = [HasNEON] in {
+  def : Pat<(v8f16 (ARMvrev32 (v8f16 QPR:$Vm))),
+            (VREV32q16 QPR:$Vm)>;
+  def : Pat<(v4f16 (ARMvrev32 (v4f16 DPR:$Vm))),
+            (VREV32d16 DPR:$Vm)>;
+}
+
 //   VREV16   : Vector Reverse elements within 16-bit halfwords
 
 class VREV16D<bits<2> op19_18, string OpcodeStr, string Dt, ValueType Ty>
   : N2V<0b11, 0b11, op19_18, 0b00, 0b00010, 0, 0, (outs DPR:$Vd),
         (ins DPR:$Vm), IIC_VMOVD,
         OpcodeStr, Dt, "$Vd, $Vm", "",
         [(set DPR:$Vd, (Ty (ARMvrev16 (Ty DPR:$Vm))))]>;
 class VREV16Q<bits<2> op19_18, string OpcodeStr, string Dt, ValueType Ty>

llvm/test/CodeGen/ARM/vrev.ll

 ; CHECK-NEXT:    vmov r0, r1, d16
 ; CHECK-NEXT:    mov pc, lr
   %tmp1 = load <4 x i16>, <4 x i16>* %A
   %tmp2 = shufflevector <4 x i16> %tmp1, <4 x i16> undef, <4 x i32> <i32 3, i32 2, i32 1, i32 0>
   ret <4 x i16> %tmp2
 }
 
 define <4 x half> @test_vrev64Df16(<4 x half>* %A) nounwind {
+; CHECK-LABEL: test_vrev64Df16:
+; CHECK:       @ %bb.0:
+; CHECK-NEXT:    vldr d16, [r0]
+; CHECK-NEXT:    vrev64.16 d16, d16
+; CHECK-NEXT:    vmov r0, r1, d16
+; CHECK-NEXT:    mov pc, lr
   %tmp1 = load <4 x half>, <4 x half>* %A
   %tmp2 = shufflevector <4 x half> %tmp1, <4 x half> undef, <4 x i32> <i32 3, i32 2, i32 1, i32 0>
   ret <4 x half> %tmp2
 }
 
 define <2 x i32> @test_vrev64D32(<2 x i32>* %A) nounwind {
 ; CHECK-LABEL: test_vrev64D32:
 ; CHECK:       @ %bb.0:

 ; CHECK-NEXT:    vmov r2, r3, d17
 ; CHECK-NEXT:    mov pc, lr
   %tmp1 = load <8 x i16>, <8 x i16>* %A
   %tmp2 = shufflevector <8 x i16> %tmp1, <8 x i16> undef, <8 x i32> <i32 3, i32 2, i32 1, i32 0, i32 7, i32 6, i32 5, i32 4>
   ret <8 x i16> %tmp2
 }
 
 define <8 x half> @test_vrev64Qf16(<8 x half>* %A) nounwind {
+; CHECK-LABEL: test_vrev64Qf16:
+; CHECK:       @ %bb.0:
+; CHECK-NEXT:    vld1.64 {d16, d17}, [r0]
+; CHECK-NEXT:    vrev64.16 q8, q8
+; CHECK-NEXT:    vmov r0, r1, d16
+; CHECK-NEXT:    vmov r2, r3, d17
+; CHECK-NEXT:    mov pc, lr
   %tmp1 = load <8 x half>, <8 x half>* %A
   %tmp2 = shufflevector <8 x half> %tmp1, <8 x half> undef, <8 x i32> <i32 3, i32 2, i32 1, i32 0, i32 7, i32 6, i32 5, i32 4>
   ret <8 x half> %tmp2
 }
 
 define <4 x i32> @test_vrev64Q32(<4 x i32>* %A) nounwind {
 ; CHECK-LABEL: test_vrev64Q32:
 ; CHECK:       @ %bb.0:

 ; CHECK-NEXT:    vmov r0, r1, d16
 ; CHECK-NEXT:    mov pc, lr
   %tmp1 = load <4 x i16>, <4 x i16>* %A
   %tmp2 = shufflevector <4 x i16> %tmp1, <4 x i16> undef, <4 x i32> <i32 1, i32 0, i32 3, i32 2>
   ret <4 x i16> %tmp2
 }
 
 define <4 x half> @test_vrev32Df16(<4 x half>* %A) nounwind {
+; CHECK-LABEL: test_vrev32Df16:
+; CHECK:       @ %bb.0:
+; CHECK-NEXT:    vldr d16, [r0]
+; CHECK-NEXT:    vrev32.16 d16, d16
+; CHECK-NEXT:    vmov r0, r1, d16
+; CHECK-NEXT:    mov pc, lr
   %tmp1 = load <4 x half>, <4 x half>* %A
   %tmp2 = shufflevector <4 x half> %tmp1, <4 x half> undef, <4 x i32> <i32 1, i32 0, i32 3, i32 2>
   ret <4 x half> %tmp2
 }
 
 define <16 x i8> @test_vrev32Q8(<16 x i8>* %A) nounwind {
 ; CHECK-LABEL: test_vrev32Q8:
 ; CHECK:       @ %bb.0:

 ; CHECK-NEXT:    vmov r2, r3, d17
 ; CHECK-NEXT:    mov pc, lr
   %tmp1 = load <8 x i16>, <8 x i16>* %A
   %tmp2 = shufflevector <8 x i16> %tmp1, <8 x i16> undef, <8 x i32> <i32 1, i32 0, i32 3, i32 2, i32 5, i32 4, i32 7, i32 6>
   ret <8 x i16> %tmp2
 }
 
 define <8 x half> @test_vrev32Qf16(<8 x half>* %A) nounwind {
+; CHECK-LABEL: test_vrev32Qf16:
+; CHECK:       @ %bb.0:
+; CHECK-NEXT:    vld1.64 {d16, d17}, [r0]
+; CHECK-NEXT:    vrev32.16 q8, q8
+; CHECK-NEXT:    vmov r0, r1, d16
+; CHECK-NEXT:    vmov r2, r3, d17
+; CHECK-NEXT:    mov pc, lr
   %tmp1 = load <8 x half>, <8 x half>* %A
   %tmp2 = shufflevector <8 x half> %tmp1, <8 x half> undef, <8 x i32> <i32 1, i32 0, i32 3, i32 2, i32 5, i32 4, i32 7, i32 6>
   ret <8 x half> %tmp2
 }
 
 define <8 x i8> @test_vrev16D8(<8 x i8>* %A) nounwind {
 ; CHECK-LABEL: test_vrev16D8:
 ; CHECK:       @ %bb.0:

 ; CHECK-NEXT:    vmov r2, r3, d17
 ; CHECK-NEXT:    mov pc, lr
   %tmp1 = load <8 x i16>, <8 x i16>* %A
   %tmp2 = shufflevector <8 x i16> %tmp1, <8 x i16> undef, <8 x i32> <i32 undef, i32 0, i32 undef, i32 2, i32 5, i32 4, i32 7, i32 undef>
   ret <8 x i16> %tmp2
 }
 
 define <8 x half> @test_vrev32Qf16_undef(<8 x half>* %A) nounwind {
+; CHECK-LABEL: test_vrev32Qf16_undef:
+; CHECK:       @ %bb.0:
+; CHECK-NEXT:    vld1.64 {d16, d17}, [r0]
+; CHECK-NEXT:    vrev32.16 q8, q8
+; CHECK-NEXT:    vmov r0, r1, d16
+; CHECK-NEXT:    vmov r2, r3, d17
+; CHECK-NEXT:    mov pc, lr
   %tmp1 = load <8 x half>, <8 x half>* %A
   %tmp2 = shufflevector <8 x half> %tmp1, <8 x half> undef, <8 x i32> <i32 undef, i32 0, i32 undef, i32 2, i32 5, i32 4, i32 7, i32 undef>
   ret <8 x half> %tmp2
 }
 
 ; A vcombine feeding a VREV should not obscure things.  Radar 8597007.
 
 define void @test_with_vcombine(<4 x float>* %v) nounwind {