[AArch64] Handle vec4 sitofp and uitofp for half

[pirama]

Summary

Set operation action for SINT_TO_FP and UINT_TO_FP nodes with v4i32
inputs to allow promotion of v4f16 results.

The conversions from v4i64 to v4f16 do not depend on this patch - v4i64
is split and the conversion gets handled while lowering v2i64. I am
adding a test here for completeness.

[pirama]

IIUC, this patch shouldn't affect v4i32 to v4f32 or v4f64 operations, but couldn't find any tests for those operations. I confirmed that locally, and can add tests with this patch if requested.

In a test of v4f16 operations (similar to the comprehensive test in http://reviews.llvm.org/D8648), only sitofp and uitofp cause problems, and will be fixed with this patch.

[pirama]

• Handled i8, i16 conversions
• Added tests for v8f16 and v16f16 tests.

[jmolloy]

Hi,

This looks fine to me, but please wait for Ahmed's LGTM.

Cheers,

James

[ab]

LGTM as well (with a couple of nits), thanks!

lib/Target/AArch64/AArch64ISelLowering.cpp

554–560

Like you did below, another comment for these might be useful, to make it clear v8iN is possible when you do ->v8f16, like v4iN is when you do ->v4f32.

test/CodeGen/AArch64/fp16-v16-instructions.ll

24–31 ↗

(On Diff #24250)

And this kind of thing is why I abhor the ARM-style NEON syntax, and much prefer Apple's.

Though conversions aren't a very good example, and you can just match the register number rather than the entire thing, like you do for S0 below. Anyway, enough ranting, I'm fine with this ;)

test/CodeGen/AArch64/fp16-v8-instructions.ll

340–341 ↗

(On Diff #24250)

What about the higher lanes? (another rant: that's why I like over-using -NEXT, it avoids this kind of question entirely).

[pirama]

test/CodeGen/AArch64/fp16-v8-instructions.ll

340–341 ↗

(On Diff #24250)

Good catch. I'll fix this.

FileCheck should support CHECK-DAG-STRICT, or something similar, and reject any line without an entry in the CHECK-DAG-STRICT block. Then we no longer have to compromise between CHECK-NEXT and hardening against schedule variations.

[pirama]

Added comment and fixed checks in a test.

[ab]

Better indeed, please commit. Thanks!

(Also, when you receive a conditional LGTM like this, and agree with all the proposed changes, I don't think there's need for another review)

lib/Target/AArch64/AArch64ISelLowering.cpp

if (Subtarget->hasNEON()) {
     setOperationAction(ISD::FP_TO_SINT, MVT::v1i64, Expand);
     setOperationAction(ISD::FP_TO_UINT, MVT::v1i64, Expand);
     setOperationAction(ISD::SINT_TO_FP, MVT::v1i64, Expand);
     setOperationAction(ISD::UINT_TO_FP, MVT::v1i64, Expand);
     setOperationAction(ISD::FP_ROUND, MVT::v1f64, Expand);
 
     setOperationAction(ISD::MUL, MVT::v1i64, Expand);
 
     // AArch64 doesn't have a direct vector ->f32 conversion instructions for
     // elements smaller than i32, so promote the input to i32 first.
     setOperationAction(ISD::UINT_TO_FP, MVT::v4i8, Promote);
     setOperationAction(ISD::SINT_TO_FP, MVT::v4i8, Promote);
     setOperationAction(ISD::UINT_TO_FP, MVT::v4i16, Promote);
     setOperationAction(ISD::SINT_TO_FP, MVT::v4i16, Promote);
     // Similarly, there is no direct i32 -> f64 vector conversion instruction.
     setOperationAction(ISD::SINT_TO_FP, MVT::v2i32, Custom);
     setOperationAction(ISD::UINT_TO_FP, MVT::v2i32, Custom);
     setOperationAction(ISD::SINT_TO_FP, MVT::v2i64, Custom);
     setOperationAction(ISD::UINT_TO_FP, MVT::v2i64, Custom);
+    // Or, direct i32 -> f16 vector conversion.  Set it so custom, so the
+    // conversion happens in two steps: v4i32 -> v4f32 -> v4f16
+    setOperationAction(ISD::SINT_TO_FP, MVT::v4i32, Custom);
+    setOperationAction(ISD::UINT_TO_FP, MVT::v4i32, Custom);
 
     // AArch64 doesn't have MUL.2d:
     setOperationAction(ISD::MUL, MVT::v2i64, Expand);
     // Custom handling for some quad-vector types to detect MULL.
     setOperationAction(ISD::MUL, MVT::v8i16, Custom);
     setOperationAction(ISD::MUL, MVT::v4i32, Custom);
     setOperationAction(ISD::MUL, MVT::v2i64, Custom);
 

test/CodeGen/AArch64/fp16-v4-instructions.ll

 }
 
 define <4 x i16> @bitcast_h_to_i(float, <4 x half> %a) {
 ; CHECK-LABEL: bitcast_h_to_i:
 ; CHECK: mov v0.16b, v1.16b
   %2 = bitcast <4 x half> %a to <4 x i16>
   ret <4 x i16> %2
 }
+
+
+define <4 x half> @sitofp_i32(<4 x i32> %a) {
+; CHECK-LABEL: sitofp_i32:
+; CHECK: scvtf [[OP1:v[0-9]+\.4s]], v0.4s
+; CHECK: fcvtn v0.4h, [[OP1]]
+  %1 = sitofp <4 x i32> %a to <4 x half>
+  ret <4 x half> %1
+}
+
+
+define <4 x half> @sitofp_i64(<4 x i64> %a) {
+; CHECK-LABEL: sitofp_i64:
+; CHECK: scvtf [[OP1:v[0-9]+\.2d]], v0.2d
+; CHECK-DAG: scvtf [[OP2:v[0-9]+\.2d]], v1.2d
+; CHECK-DAG: fcvtn [[OP3:v[0-9]+]].2s, [[OP1]]
+; CHECK: fcvtn2 [[OP3]].4s, [[OP2]]
+; CHECK: fcvtn v0.4h, [[OP3]].4s
+  %1 = sitofp <4 x i64> %a to <4 x half>
+  ret <4 x half> %1
+}
+
+
+define <4 x half> @uitofp_i32(<4 x i32> %a) {
+; CHECK-LABEL: uitofp_i32:
+; CHECK: ucvtf [[OP1:v[0-9]+\.4s]], v0.4s
+; CHECK: fcvtn v0.4h, [[OP1]]
+  %1 = uitofp <4 x i32> %a to <4 x half>
+  ret <4 x half> %1
+}
+
+
+define <4 x half> @uitofp_i64(<4 x i64> %a) {
+; CHECK-LABEL: uitofp_i64:
+; CHECK: ucvtf [[OP1:v[0-9]+\.2d]], v0.2d
+; CHECK-DAG: ucvtf [[OP2:v[0-9]+\.2d]], v1.2d
+; CHECK-DAG: fcvtn [[OP3:v[0-9]+]].2s, [[OP1]]
+; CHECK: fcvtn2 [[OP3]].4s, [[OP2]]
+; CHECK: fcvtn v0.4h, [[OP3]].4s
+  %1 = uitofp <4 x i64> %a to <4 x half>
+  ret <4 x half> %1
+}