[AArch64] PR28877: Don't assume we're running after legalization when creating vcvtfp2fxs

[sbaranga]

Summary

The DAG combine transformation that was generating the
aarch64_neon_vcvtfp2fxs node was assuming that all
inputs where legal and wasn't accounting that the input
could be a v4f64 if we're trying to do the transformation
before legalization. We now bail out in this case.

All illegal types besides v4f64 were already rejected.

Fixes https://llvm.org/bugs/show_bug.cgi?id=28877.

[jmolloy]

LGTM!

[sbaranga]

Thanks, James! Committed in r278002.

lib/Target/AArch64/AArch64ISelLowering.cpp

static SDValue performIntToFpCombine(SDNode *N, SelectionDAG &DAG,
   }
 
   return SDValue();
 }
 
 /// Fold a floating-point multiply by power of two into floating-point to
 /// fixed-point conversion.
 static SDValue performFpToIntCombine(SDNode *N, SelectionDAG &DAG,
+                                     TargetLowering::DAGCombinerInfo &DCI,
                                      const AArch64Subtarget *Subtarget) {
   if (!Subtarget->hasNEON())
     return SDValue();
 
   SDValue Op = N->getOperand(0);
   if (!Op.getValueType().isVector() || !Op.getValueType().isSimple() ||
       Op.getOpcode() != ISD::FMUL)
     return SDValue();
static SDValue performFpToIntCombine(SDNode *N, SelectionDAG &DAG,
   unsigned NumLanes = Op.getValueType().getVectorNumElements();
   switch (NumLanes) {
   default:
     return SDValue();
   case 2:
     ResTy = FloatBits == 32 ? MVT::v2i32 : MVT::v2i64;
     break;
   case 4:
-    ResTy = MVT::v4i32;
+    ResTy = FloatBits == 32 ? MVT::v4i32 : MVT::v4i64;
     break;
   }
 
+  if (ResTy == MVT::v4i64 && DCI.isBeforeLegalizeOps())
+    return SDValue();
+
+  assert((ResTy != MVT::v4i64 || DCI.isBeforeLegalizeOps()) &&
+         "Illegal vector type after legalization");
+
   SDLoc DL(N);
   bool IsSigned = N->getOpcode() == ISD::FP_TO_SINT;
   unsigned IntrinsicOpcode = IsSigned ? Intrinsic::aarch64_neon_vcvtfp2fxs
                                       : Intrinsic::aarch64_neon_vcvtfp2fxu;
   SDValue FixConv =
       DAG.getNode(ISD::INTRINSIC_WO_CHAIN, DL, ResTy,
                   DAG.getConstant(IntrinsicOpcode, DL, MVT::i32),
                   Op->getOperand(0), DAG.getConstant(C, DL, MVT::i32));
case ISD::XOR:
     return performXorCombine(N, DAG, DCI, Subtarget);
   case ISD::MUL:
     return performMulCombine(N, DAG, DCI, Subtarget);
   case ISD::SINT_TO_FP:
   case ISD::UINT_TO_FP:
     return performIntToFpCombine(N, DAG, Subtarget);
   case ISD::FP_TO_SINT:
   case ISD::FP_TO_UINT:
-    return performFpToIntCombine(N, DAG, Subtarget);
+    return performFpToIntCombine(N, DAG, DCI, Subtarget);
   case ISD::FDIV:
     return performFDivCombine(N, DAG, Subtarget);
   case ISD::OR:
     return performORCombine(N, DCI, Subtarget);
   case ISD::SRL:
     return performSRLCombine(N, DCI);
   case ISD::INTRINSIC_WO_CHAIN:
     return performIntrinsicCombine(N, DCI, Subtarget);

test/CodeGen/AArch64/aarch64-vcvtfp2fxs-combine.ll

+; RUN: llc < %s -mtriple=aarch64-linux-eabi -o - | FileCheck %s
+
+%struct.a= type { i64, i64, i64, i64 }
+
+; DAG combine will try to perform a transformation that  creates a vcvtfp2fxs
+; with a v4f64 input. Since v4i64 is not legal we should bail out. We can
+; pottentially still create the vcvtfp2fxs node after legalization (but on a
+; v2f64).
+
+; CHECK-LABEL: fun1
+define void @fun1() local_unnamed_addr {
+entry:
+  %mul = fmul <4 x double> zeroinitializer, <double 6.553600e+04, double 6.553600e+04, double 6.553600e+04, double 6.553600e+04>
+  %toi = fptosi <4 x double> %mul to <4 x i64>
+  %ptr = getelementptr inbounds %struct.a, %struct.a* undef, i64 0, i32 2
+  %elem = extractelement <4 x i64> %toi, i32 1
+  store i64 %elem, i64* %ptr, align 8
+  call void @llvm.trap()
+  unreachable
+}
+
+; Function Attrs: noreturn nounwind
+declare void @llvm.trap()
+