[AArch64] Add fallback in FastISel fp16 conversions

[ijsung]

Summary

• Fix assertion failures on F16 to/from int types in FastISel by falling back to regular ISel
• Add a testcase of various conversion cases with FastISel (-O0)

[ijsung]

Revise the testcase with FileCheck label name workarounds

Apparently FileCheck's CHECK-LABEL can get confused in non-obvious ways
if a label happen to be a prefix of other labels in some cases.

Revise label names of the tests to avoid confusing FileCheck; add a few
more checks.

[kristof.beyls]

lib/Target/AArch64/AArch64FastISel.cpp

2857–2860 ↗

(On Diff #100924)

It looks a bit strange to me that for selectFPToInt, the test is (SrcVT == MVT::f128 || SrcVT == MVT::f16) to fall back to DAGISel, while for selectIntToFP, the test is (DestVT == MVT::f16), not checking for a 128 bit floating point data type.
I guess this is because i128 is not a legal type for AArch64?

If my guess is correct, this change looks reasonable; otherwise I'm not entirely sure.

test/CodeGen/AArch64/arm64-fast-isel-conversion-fallback.ll

1 ↗

(On Diff #100924)

I guess that -mcpu=cortex-a57 isn't needed in this test line?
For the actual tests, I think @SjoerdMeijer probably will be able to review them better than I can.

[SjoerdMeijer]

test/CodeGen/AArch64/arm64-fast-isel-conversion-fallback.ll

1 ↗

(On Diff #100924)

The tests look very reasonable to me.

[pirama]

lib/Target/AArch64/AArch64FastISel.cpp

2857–2860 ↗

(On Diff #100924)

isTypeLegal() (called in line 2855 above) returns false for f128. So both functions have matching behavior in that they don't handle f128.

[ijsung]

Remove -mcpu=cortex-a57 in the testcase

Per suggestion from @kristof.beyls, -mcpu is not really necessary.

test/CodeGen/AArch64/arm64-fast-isel-conversion-fallback.ll

1 ↗

(On Diff #100924)

Revised the RUN line and removed the -mcpu. Thanks.

[SjoerdMeijer]

Looks good to me.

llvm/trunk/lib/Target/AArch64/AArch64FastISel.cpp

bool AArch64FastISel::selectFPToInt(const Instruction *I, bool Signed) {
   if (!isTypeLegal(I->getType(), DestVT) || DestVT.isVector())
     return false;
 
   unsigned SrcReg = getRegForValue(I->getOperand(0));
   if (SrcReg == 0)
     return false;
 
   EVT SrcVT = TLI.getValueType(DL, I->getOperand(0)->getType(), true);
-  if (SrcVT == MVT::f128)
+  if (SrcVT == MVT::f128 || SrcVT == MVT::f16)
     return false;
 
   unsigned Opc;
   if (SrcVT == MVT::f64) {
     if (Signed)
       Opc = (DestVT == MVT::i32) ? AArch64::FCVTZSUWDr : AArch64::FCVTZSUXDr;
     else
       Opc = (DestVT == MVT::i32) ? AArch64::FCVTZUUWDr : AArch64::FCVTZUUXDr;
bool AArch64FastISel::selectFPToInt(const Instruction *I, bool Signed) {
   updateValueMap(I, ResultReg);
   return true;
 }
 
 bool AArch64FastISel::selectIntToFP(const Instruction *I, bool Signed) {
   MVT DestVT;
   if (!isTypeLegal(I->getType(), DestVT) || DestVT.isVector())
     return false;
+  // Let regular ISEL handle FP16
+  if (DestVT == MVT::f16)
+    return false;
+
   assert((DestVT == MVT::f32 || DestVT == MVT::f64) &&
          "Unexpected value type.");
 
   unsigned SrcReg = getRegForValue(I->getOperand(0));
   if (!SrcReg)
     return false;
   bool SrcIsKill = hasTrivialKill(I->getOperand(0));
 

llvm/trunk/test/CodeGen/AArch64/arm64-fast-isel-conversion-fallback.ll

+; RUN: llc -O0 -verify-machineinstrs -mtriple=arm64-eabi < %s | FileCheck --enable-var-scope %s
+
+; Test fptosi
+define i32 @fptosi_wh(half %a) nounwind ssp {
+entry:
+; CHECK-LABEL: fptosi_wh
+; CHECK: fcvt s1, h0
+; CHECK: fcvtzs [[REG:w[0-9]+]], s1
+; CHECK: mov w0, [[REG]]
+  %conv = fptosi half %a to i32
+  ret i32 %conv
+}
+
+; Test fptoui
+define i32 @fptoui_swh(half %a) nounwind ssp {
+entry:
+; CHECK-LABEL: fptoui_swh
+; CHECK: fcvt s1, h0
+; CHECK: fcvtzu [[REG:w[0-9]+]], s1
+; CHECK: mov w0, [[REG]]
+  %conv = fptoui half %a to i32
+  ret i32 %conv
+}
+
+; Test sitofp
+define half @sitofp_hw_i1(i1 %a) nounwind ssp {
+entry:
+; CHECK-LABEL: sitofp_hw_i1
+; CHECK: sbfx w0, w0, #0, #1
+; CHECK: scvtf s0, w0
+; CHECK: fcvt  h0, s0
+  %conv = sitofp i1 %a to half
+  ret half %conv
+}
+
+; Test sitofp
+define half @sitofp_hw_i8(i8 %a) nounwind ssp {
+entry:
+; CHECK-LABEL: sitofp_hw_i8
+; CHECK: sxtb w0, w0
+; CHECK: scvtf s0, w0
+; CHECK: fcvt  h0, s0
+  %conv = sitofp i8 %a to half
+  ret half %conv
+}
+
+; Test sitofp
+define half @sitofp_hw_i16(i16 %a) nounwind ssp {
+entry:
+; CHECK-LABEL: sitofp_hw_i16
+; CHECK: sxth w0, w0
+; CHECK: scvtf s0, w0
+; CHECK: fcvt  h0, s0
+  %conv = sitofp i16 %a to half
+  ret half %conv
+}
+
+; Test sitofp
+define half @sitofp_hw_i32(i32 %a) nounwind ssp {
+entry:
+; CHECK-LABEL: sitofp_hw_i32
+; CHECK: scvtf s0, w0
+; CHECK: fcvt  h0, s0
+  %conv = sitofp i32 %a to half
+  ret half %conv
+}
+
+; Test sitofp
+define half @sitofp_hx(i64 %a) nounwind ssp {
+entry:
+; CHECK-LABEL: sitofp_hx
+; CHECK: scvtf s0, x0
+; CHECK: fcvt  h0, s0
+  %conv = sitofp i64 %a to half
+  ret half %conv
+}
+
+; Test uitofp
+define half @uitofp_hw_i1(i1 %a) nounwind ssp {
+entry:
+; CHECK-LABEL: uitofp_hw_i1
+; CHECK: and w0, w0, #0x1
+; CHECK: ucvtf s0, w0
+; CHECK: fcvt  h0, s0
+  %conv = uitofp i1 %a to half
+  ret half %conv
+}
+
+; Test uitofp
+define half @uitofp_hw_i8(i8 %a) nounwind ssp {
+entry:
+; CHECK-LABEL: uitofp_hw_i8
+; CHECK: and w0, w0, #0xff
+; CHECK: ucvtf s0, w0
+; CHECK: fcvt  h0, s0
+  %conv = uitofp i8 %a to half
+  ret half %conv
+}
+
+; Test uitofp
+define half @uitofp_hw_i16(i16 %a) nounwind ssp {
+entry:
+; CHECK-LABEL: uitofp_hw_i16
+; CHECK: and w0, w0, #0xffff
+; CHECK: ucvtf s0, w0
+; CHECK: fcvt  h0, s0
+  %conv = uitofp i16 %a to half
+  ret half %conv
+}
+
+; Test uitofp
+define half @uitofp_hw_i32(i32 %a) nounwind ssp {
+entry:
+; CHECK-LABEL: uitofp_hw_i32
+; CHECK: ucvtf s0, w0
+; CHECK: fcvt  h0, s0
+  %conv = uitofp i32 %a to half
+  ret half %conv
+}
+
+; Test uitofp
+define half @uitofp_hx(i64 %a) nounwind ssp {
+entry:
+; CHECK-LABEL: uitofp_hx
+; CHECK: ucvtf s0, x0
+; CHECK: fcvt  h0, s0
+  %conv = uitofp i64 %a to half
+  ret half %conv
+}
+
+