Fix float literals under XO & fp-armv8

[christof]

Summary

When targeting execute-only and fp-armv8, float constants in a compare resulted
in instruction selection failures in some cases. This is now fixed by using
vmov.f32 where possible, otherwise the floating point constant is lowered into
a integer constant that is moved into a floating point register.

This patch also restores using fpcmp with immediate 0 when fp-armv8 is specified.

[SjoerdMeijer]

lib/Target/ARM/ARMISelLowering.cpp

5961–5977

Sorry to be a bit of a pain here, but I think that after my FP16 changes went in, we can also expect f16 values here. But hopefully that does not changes much; looks like only thing is that instead of a VMOVSR, we need a VMOVHR (and a test).

[christof]

Updated the change to take into account f16 types. I cannot test it yet, as f16 seems to be incomplete. I've reported this to SjoerdMeijer already, and he is fixing f16. I am wondering if we can commit this and once f16 constants are actually lowered, update the test as part of that change.

[SjoerdMeijer]

My remark about f16 values, and trying to trigger the assert, actually revealed some problems in lowering f16 constants. I am working on a fix now, but I don't see the point of holding back this patch. So this looks good to me, and if you put the assert back before committing, then we can pick up execute-only and f16 values later.

[christof]

Committed as r328313

[christof]

Follow up patch at https://reviews.llvm.org/D44941

lib/Target/ARM/ARMISelLowering.h

enum NodeType : unsigned {
 
       ADDC,         // Add with carry
       ADDE,         // Add using carry
       SUBC,         // Sub with carry
       SUBE,         // Sub using carry
 
       VMOVRRD,      // double to two gprs.
       VMOVDRR,      // Two gprs to double.
+      VMOVSR,       // move gpr to single, used for f32 literal constructed in a gpr
 
       EH_SJLJ_SETJMP,         // SjLj exception handling setjmp.
       EH_SJLJ_LONGJMP,        // SjLj exception handling longjmp.
       EH_SJLJ_SETUP_DISPATCH, // SjLj exception handling setup_dispatch.
 
       TC_RETURN,    // Tail call return pseudo.
 
       THREAD_POINTER,

lib/Target/ARM/ARMISelLowering.cpp

const char *ARMTargetLowering::getTargetNodeName(unsigned Opcode) const {
   case ARMISD::ADDE:          return "ARMISD::ADDE";
   case ARMISD::SUBC:          return "ARMISD::SUBC";
   case ARMISD::SUBE:          return "ARMISD::SUBE";
 
   case ARMISD::VMOVRRD:       return "ARMISD::VMOVRRD";
   case ARMISD::VMOVDRR:       return "ARMISD::VMOVDRR";
   case ARMISD::VMOVhr:        return "ARMISD::VMOVhr";
   case ARMISD::VMOVrh:        return "ARMISD::VMOVrh";
+  case ARMISD::VMOVSR:        return "ARMISD::VMOVSR";
 
   case ARMISD::EH_SJLJ_SETJMP: return "ARMISD::EH_SJLJ_SETJMP";
   case ARMISD::EH_SJLJ_LONGJMP: return "ARMISD::EH_SJLJ_LONGJMP";
   case ARMISD::EH_SJLJ_SETUP_DISPATCH: return "ARMISD::EH_SJLJ_SETUP_DISPATCH";
 
   case ARMISD::TC_RETURN:     return "ARMISD::TC_RETURN";
 
   case ARMISD::THREAD_POINTER:return "ARMISD::THREAD_POINTER";
if (LHS.getValueType() == MVT::i32) {
     SDValue Cmp = getARMCmp(LHS, RHS, CC, ARMcc, DAG, dl);
     return getCMOV(dl, VT, FalseVal, TrueVal, ARMcc, CCR, Cmp, DAG);
   }
 
   ARMCC::CondCodes CondCode, CondCode2;
   bool InvalidOnQNaN;
   FPCCToARMCC(CC, CondCode, CondCode2, InvalidOnQNaN);
 
-  // Try to generate VMAXNM/VMINNM on ARMv8.
-  if (Subtarget->hasFPARMv8() && (TrueVal.getValueType() == MVT::f32 ||
-                                  TrueVal.getValueType() == MVT::f64)) {
+  // Try to generate VMAXNM/VMINNM on ARMv8. Except if we compare to a zero.
+  // This ensures we use CMPFPw0 instead of CMPFP in such case.
+  if (Subtarget->hasFPARMv8() && !isFloatingPointZero(RHS) &&
+    (TrueVal.getValueType() == MVT::f32 || TrueVal.getValueType() == MVT::f64)) {
     bool swpCmpOps = false;
     bool swpVselOps = false;
     checkVSELConstraints(CC, CondCode, swpCmpOps, swpVselOps);
 
     if (CondCode == ARMCC::GT || CondCode == ARMCC::GE ||
         CondCode == ARMCC::VS || CondCode == ARMCC::EQ) {
       if (swpCmpOps)
         std::swap(LHS, RHS);
static SDValue isNEONModifiedImm(uint64_t SplatBits, uint64_t SplatUndef,
   }
 
   unsigned EncodedVal = ARM_AM::createNEONModImm(OpCmode, Imm);
   return DAG.getTargetConstant(EncodedVal, dl, MVT::i32);
 }
 
 SDValue ARMTargetLowering::LowerConstantFP(SDValue Op, SelectionDAG &DAG,
                                            const ARMSubtarget *ST) const {
-  bool IsDouble = Op.getValueType() == MVT::f64;
+  EVT VT = Op.getValueType();
+  bool IsDouble = (VT == MVT::f64);
   ConstantFPSDNode *CFP = cast<ConstantFPSDNode>(Op);
   const APFloat &FPVal = CFP->getValueAPF();
 
   // Prevent floating-point constants from using literal loads
   // when execute-only is enabled.
   if (ST->genExecuteOnly()) {
+    // If we can represent the constant as an immediate, don't lower it
+    if (isFPImmLegal(FPVal, VT))
+      return Op;
+    // Otherwise, construct as integer, and move to float register
     APInt INTVal = FPVal.bitcastToAPInt();
     SDLoc DL(CFP);
-    if (IsDouble) {
+    switch (VT.getSimpleVT().SimpleTy) {
+      default:
+        llvm_unreachable("Unknown floating point type!");
+        break;
+      case MVT::f64: {
         SDValue Lo = DAG.getConstant(INTVal.trunc(32), DL, MVT::i32);
         SDValue Hi = DAG.getConstant(INTVal.lshr(32).trunc(32), DL, MVT::i32);
         if (!ST->isLittle())
           std::swap(Lo, Hi);
         return DAG.getNode(ARMISD::VMOVDRR, DL, MVT::f64, Lo, Hi);
-    } else {
-      return DAG.getConstant(INTVal, DL, MVT::i32);
+      }
+      case MVT::f32:
+          return DAG.getNode(ARMISD::VMOVSR, DL, VT,
+              DAG.getConstant(INTVal, DL, MVT::i32));
+      case MVT::f16:
+          return DAG.getNode(ARMISD::VMOVhr, DL, VT,
+              DAG.getConstant(INTVal, DL, MVT::i32));
     }
   }
 
   if (!ST->hasVFP3())
     return SDValue();
 
   // Use the default (constant pool) lowering for double constants when we have
   // an SP-only FPU

lib/Target/ARM/ARMInstrVFP.td

 //===----------------------------------------------------------------------===//
 
 def SDT_CMPFP0  : SDTypeProfile<0, 2, [SDTCisFP<0>, SDTCisVT<1, i32>]>;
 def SDT_VMOVDRR : SDTypeProfile<1, 2, [SDTCisVT<0, f64>, SDTCisVT<1, i32>,
                                        SDTCisSameAs<1, 2>]>;
 def SDT_VMOVRRD : SDTypeProfile<2, 1, [SDTCisVT<0, i32>, SDTCisSameAs<0, 1>,
                                        SDTCisVT<2, f64>]>;
 
+def SDT_VMOVSR : SDTypeProfile<1, 1, [SDTCisVT<0, f32>, SDTCisVT<1, i32>]>;
+
 def arm_fmstat : SDNode<"ARMISD::FMSTAT",  SDTNone, [SDNPInGlue, SDNPOutGlue]>;
 def arm_cmpfp  : SDNode<"ARMISD::CMPFP",   SDT_ARMFCmp, [SDNPOutGlue]>;
 def arm_cmpfp0 : SDNode<"ARMISD::CMPFPw0", SDT_CMPFP0, [SDNPOutGlue]>;
 def arm_fmdrr  : SDNode<"ARMISD::VMOVDRR", SDT_VMOVDRR>;
 def arm_fmrrd  : SDNode<"ARMISD::VMOVRRD", SDT_VMOVRRD>;
+def arm_vmovsr  : SDNode<"ARMISD::VMOVSR", SDT_VMOVSR>;
 
 def SDT_VMOVhr : SDTypeProfile<1, 1, [SDTCisFP<0>, SDTCisVT<1, i32>] >;
 def SDT_VMOVrh : SDTypeProfile<1, 1, [SDTCisVT<0, i32>, SDTCisFP<1>] >;
 def arm_vmovhr : SDNode<"ARMISD::VMOVhr", SDT_VMOVhr>;
 def arm_vmovrh : SDNode<"ARMISD::VMOVrh", SDT_VMOVrh>;
 
 //===----------------------------------------------------------------------===//
 // Operand Definitions.
def VMOVSR : AVConv4I<0b11100000, 0b1010,
 
   let Inst{6-5}   = 0b00;
   let Inst{3-0}   = 0b0000;
 
   // Some single precision VFP instructions may be executed on both NEON and VFP
   // pipelines.
   let D = VFPNeonDomain;
 }
+def : Pat<(arm_vmovsr GPR:$Rt), (VMOVSR GPR:$Rt)>;
 
 let hasSideEffects = 0 in {
 def VMOVRRD  : AVConv3I<0b11000101, 0b1011,
                         (outs GPR:$Rt, GPR:$Rt2), (ins DPR:$Dm),
                         IIC_fpMOVDI, "vmov", "\t$Rt, $Rt2, $Dm",
                  [(set GPR:$Rt, GPR:$Rt2, (arm_fmrrd DPR:$Dm))]>,
                Sched<[WriteFPMOV]> {
   // Instruction operands.

test/CodeGen/ARM/fcmp-xo.ll

+; RUN: llc -mtriple=thumbv7m-arm-none-eabi -mattr=+execute-only,+fp-armv8 %s -o - | FileCheck %s
+
+; This function used to run into a code selection error on fp-armv8 due to
+; different ordering of the constant arguments of fcmp. Fixed by extending the
+; code selection to handle the missing case.
+define arm_aapcs_vfpcc void @foo0() local_unnamed_addr {
+  br i1 undef, label %.end, label %1
+
+  %2 = fcmp nsz olt float undef, 0.000000e+00
+  %3 = select i1 %2, float -5.000000e-01, float 5.000000e-01
+  %4 = fadd nsz float undef, %3
+  %5 = fptosi float %4 to i32
+  %6 = ashr i32 %5, 4
+  %7 = icmp slt i32 %6, 0
+  br i1 %7, label %8, label %.end
+
+  tail call arm_aapcs_vfpcc void @bar()
+  br label %.end
+
+.end:
+  ret void
+}
+; CHECK-LABEL: foo0
+; CHECK: vcmpe.f32 {{s[0-9]+}}, #0
+
+
+define arm_aapcs_vfpcc void @float1() local_unnamed_addr {
+  br i1 undef, label %.end, label %1
+
+  %2 = fcmp nsz olt float undef, 1.000000e+00
+  %3 = select i1 %2, float -5.000000e-01, float 5.000000e-01
+  %4 = fadd nsz float undef, %3
+  %5 = fptosi float %4 to i32
+  %6 = ashr i32 %5, 4
+  %7 = icmp slt i32 %6, 0
+  br i1 %7, label %8, label %.end
+
+  tail call arm_aapcs_vfpcc void @bar()
+  br label %.end
+
+.end:
+  ret void
+}
+; CHECK-LABEL: float1
+; CHECK: vmov.f32 [[FPREG:s[0-9]+]], #1.000000e+00
+; CHECK: vcmpe.f32 [[FPREG]], {{s[0-9]+}}
+
+define arm_aapcs_vfpcc void @float128() local_unnamed_addr {
+  br i1 undef, label %.end, label %1
+
+  %2 = fcmp nsz olt float undef, 128.000000e+00
+  %3 = select i1 %2, float -5.000000e-01, float 5.000000e-01
+  %4 = fadd nsz float undef, %3
+  %5 = fptosi float %4 to i32
+  %6 = ashr i32 %5, 4
+  %7 = icmp slt i32 %6, 0
+  br i1 %7, label %8, label %.end
+
+  tail call arm_aapcs_vfpcc void @bar()
+  br label %.end
+
+.end:
+  ret void
+}
+; CHECK-LABEL: float128
+; CHECK: mov.w [[REG:r[0-9]+]], #1124073472
+; CHECK: vmov [[FPREG:s[0-9]+]], [[REG]]
+; CHECK: vcmpe.f32 [[FPREG]], {{s[0-9]+}}
+
+
+define arm_aapcs_vfpcc void @double1() local_unnamed_addr {
+  br i1 undef, label %.end, label %1
+
+  %2 = fcmp nsz olt double undef, 1.000000e+00
+  %3 = select i1 %2, double -5.000000e-01, double 5.000000e-01
+  %4 = fadd nsz double undef, %3
+  %5 = fptosi double %4 to i32
+  %6 = ashr i32 %5, 4
+  %7 = icmp slt i32 %6, 0
+  br i1 %7, label %8, label %.end
+
+  tail call arm_aapcs_vfpcc void @bar()
+  br label %.end
+
+.end:
+  ret void
+}
+; CHECK-LABEL: double1
+; CHECK: vmov.f64 [[FPREG:d[0-9]+]], #1.000000e+00
+; CHECK: vcmpe.f64 [[FPREG]], {{d[0-9]+}}
+
+define arm_aapcs_vfpcc void @double128() local_unnamed_addr {
+  br i1 undef, label %.end, label %1
+
+  %2 = fcmp nsz olt double undef, 128.000000e+00
+  %3 = select i1 %2, double -5.000000e-01, double 5.000000e-01
+  %4 = fadd nsz double undef, %3
+  %5 = fptosi double %4 to i32
+  %6 = ashr i32 %5, 4
+  %7 = icmp slt i32 %6, 0
+  br i1 %7, label %8, label %.end
+
+  tail call arm_aapcs_vfpcc void @bar()
+  br label %.end
+
+.end:
+  ret void
+}
+; CHECK-LABEL: double128
+; CHECK: movs [[REGL:r[0-9]+]], #0
+; CHECK: movs [[REGH:r[0-9]+]], #0
+; CHECK: movt [[REGH]], #16480
+; CHECK: vmov [[FPREG:d[0-9]+]], [[REGL]], [[REGH]]
+; CHECK: vcmpe.f64 [[FPREG]], {{d[0-9]+}}
+
+
+declare arm_aapcs_vfpcc void @bar() local_unnamed_addr
+