Do not ASSERTZEXT for i16 result of bitcast from f16 operand

[pirama]

Summary

During legalization if i16, do not ASSERTZEXT the result of FP_TO_FP16.
Directly return an FP_TO_FP16 node with return type as the
promote-to-type of i16.

This patch also removes extraneous length check. This legalization
should be valid even if integer and float types are of different
lengths.

This patch breaks a hard-float test for fp16 args. The test is changed
to allow a vmov to zero-out the top bits, and also ensure that the
return value is in an FP register.

[jmolloy]

Hi Pirama,

Can you explain why this is necessary? Why is ASSERTZEXT not correct in this case?

James

[pirama]

Hi James,

Can you explain why this is necessary? Why is ASSERTZEXT not correct in this case?

ASSERTZEXT does not generate any instructions but just records that its input operand is zero extended. When I wrote this code initially, I incorrectly inserted this ASSERTZEXT while extending the result from i16 to i32. That later causes the backend to assume that the top bits are zero and incorrectly use the 'orr' instruction (https://llvm.org/bugs/show_bug.cgi?id=25492). Removing the ASSERTZEXT forces the backend to use uxth and pkhbt.

llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp

if (NOutVT.bitsEq(NInVT) && !NOutVT.isVector() && !NInVT.isVector())
       // The input promotes to the same size.  Convert the promoted value.
       return DAG.getNode(ISD::BITCAST, dl, NOutVT, GetPromotedInteger(InOp));
     break;
   case TargetLowering::TypeSoftenFloat:
     // Promote the integer operand by hand.
     return DAG.getNode(ISD::ANY_EXTEND, dl, NOutVT, GetSoftenedFloat(InOp));
   case TargetLowering::TypePromoteFloat: {
     // Convert the promoted float by hand.
-    if (NOutVT.bitsEq(NInVT)) {
-      SDValue PromotedOp = GetPromotedFloat(InOp);
-      SDValue Trunc = DAG.getNode(ISD::FP_TO_FP16, dl, NOutVT, PromotedOp);
-      return DAG.getNode(ISD::AssertZext, dl, NOutVT, Trunc,
-                         DAG.getValueType(OutVT));
-    }
+    return DAG.getNode(ISD::FP_TO_FP16, dl, NOutVT, PromotedOp);
     break;
   }
   case TargetLowering::TypeExpandInteger:
   case TargetLowering::TypeExpandFloat:
     break;
   case TargetLowering::TypeScalarizeVector:
     // Convert the element to an integer and promote it by hand.
     if (!NOutVT.isVector())

llvm/trunk/test/CodeGen/ARM/fp16-args.ll

 ; SOFT: vcvtb.f16.f32   {{s[0-9]+}}, {{s[0-9]+}}
 ; SOFT: vmov    r0, {{s[0-9]+}}
 
 ; HARD-NOT: vmov
 ; HARD-NOT: uxth
 ; HARD: vcvtb.f32.f16   {{s[0-9]+}}, s1
 ; HARD: vcvtb.f32.f16   {{s[0-9]+}}, s0
 ; HARD: vadd.f32        {{s[0-9]+}}, {{s[0-9]+}}, {{s[0-9]+}}
-; HARD: vcvtb.f16.f32   s0, {{s[0-9]+}}
-; HARD-NOT: vmov
-; HARD-NOT: uxth
+; HARD: vcvtb.f16.f32   [[SREG:s[0-9]+]], {{s[0-9]+}}
+; HARD-NEXT: vmov            [[REG0:r[0-9]+]], [[SREG]]
+; HARD-NEXT: uxth            [[REG1:r[0-9]+]], [[REG0]]
+; HARD-NEXT: vmov            s0, [[REG1]]
 
 ; CHECK: bx lr
 }

llvm/trunk/test/CodeGen/ARM/fp16-v3.ll

+; RUN: llc -mattr=+fp16 < %s | FileCheck %s --check-prefix=CHECK
+
+target datalayout = "e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64"
+target triple = "armv7a--none-eabi"
+
+; CHECK-LABEL: test_vec3:
+; CHECK: vcvtb.f32.f16
+; CHECK: vcvt.f32.s32
+; CHECK: vadd.f32
+; CHECK-NEXT: vcvtb.f16.f32 [[SREG:s[0-9]+]], {{.*}}
+; CHECK-NEXT: vmov [[RREG1:r[0-9]+]], [[SREG]]
+; CHECK-NEXT: uxth [[RREG2:r[0-9]+]], [[RREG1]]
+; CHECK-NEXT: pkhbt [[RREG3:r[0-9]+]], [[RREG1]], [[RREG1]], lsl #16
+; CHECK-DAG: strh [[RREG1]], [r0, #4]
+; CHECK-DAG: vmov [[DREG:d[0-9]+]], [[RREG3]], [[RREG2]]
+; CHECK-DAG: vst1.32 {[[DREG]][0]}, [r0:32]
+; CHECK-NEXT: bx lr
+define void @test_vec3(<3 x half>* %arr, i32 %i) #0 {
+  %H = sitofp i32 %i to half
+  %S = fadd half %H, 0xH4A00
+  %1 = insertelement <3 x half> undef, half %S, i32 0
+  %2 = insertelement <3 x half> %1, half %S, i32 1
+  %3 = insertelement <3 x half> %2, half %S, i32 2
+  store <3 x half> %3, <3 x half>* %arr, align 8
+  ret void
+}
+
+attributes #0 = { nounwind }