[Sparc] efficient pattern for UINT_TO_FP conversion

[avl]

Summary

while investigating performance degradation of imagick benchmark
there were found inefficient pattern for UINT_TO_FP conversion.
That pattern causes RAW hazard in assembly code. Specifically,
uitofp IR operator results in poor assembler :

st          %i0, [%fp - 952]
ldd         [%fp - 952], %f0 

it stores 32-bit integer register into memory location and then 
loads 64-bit floating point data from that location. 
That is exactly RAW hazard case. To optimize that case it is 
possible to use SPISD::ITOF and SPISD::XTOF for conversion from 
integer to floating point data type and to use ISD::BITCAST to 
copy from integer register into floating point register.  
The fix is to write custom UINT_TO_FP pattern using SPISD::ITOF,
SPISD::XTOF, ISD::BITCAST.

[avl]

Could someone review this fix, please ?

[avl]

ping...

[jyknight]

Maybe a test for bitcast from int to float, without the uitofp, too?

lib/Target/Sparc/SparcISelLowering.cpp

1392

Move both these functions down below with the rest of the Lower* members (near where LowerUINT_TO_FP was previously) -- this section is for the calling convention lowering.

test/CodeGen/SPARC/uint_to_fp.ll

1

Tests for i64?
Tests for converting to float?

Do these overlap with some tests from "float.ll"? (If so, seems reasonable to pull those out here, too.)

test/CodeGen/SPARC/vis3.ll

1

Merge with the previous file?

[avl]

updated a fix according to comments, specifically:

• moved down LowerBITCAST, LowerUINT_TO_FP
• moved tests into float.ll(it already has test for i64)
• added test for bitcast from int to float(without uitofp)
• added test for uitofp for float

[avl]

Hi, Is the fix good now?

[jyknight]

Seems reasonable.

[avl]

James, Fedor, Could you integrate this ? I do not have commit rights yet.

Thank you, Alexey.

[fedor.sergeev]

pushed.
Thanks for keeping up with it.

lib/Target/Sparc/SparcISelLowering.h

public:
     SDValue LowerF128Op(SDValue Op, SelectionDAG &DAG,
                         const char *LibFuncName,
                         unsigned numArgs) const;
     SDValue LowerF128Compare(SDValue LHS, SDValue RHS, unsigned &SPCC,
                              const SDLoc &DL, SelectionDAG &DAG) const;
 
     SDValue LowerINTRINSIC_WO_CHAIN(SDValue Op, SelectionDAG &DAG) const;
 
+    SDValue LowerBITCAST(SDValue Op, SelectionDAG &DAG) const;
+
+    SDValue LowerUINT_TO_FP(SDValue Op, SelectionDAG &DAG) const;
+
     bool ShouldShrinkFPConstant(EVT VT) const override {
       // Do not shrink FP constpool if VT == MVT::f128.
       // (ldd, call _Q_fdtoq) is more expensive than two ldds.
       return VT != MVT::f128;
     }
 
     bool shouldInsertFencesForAtomic(const Instruction *I) const override {
       // FIXME: We insert fences for each atomics and generate

lib/Target/Sparc/SparcISelLowering.cpp

if (VA.isExtInLoc())
       RV = DAG.getNode(ISD::TRUNCATE, DL, VA.getValVT(), RV);
 
     InVals.push_back(RV);
   }
 
   return Chain;
 }
 
+SDValue SparcTargetLowering::LowerBITCAST(SDValue Op, SelectionDAG &DAG) const {
+  SDLoc dl(Op);
+  EVT SrcVT = Op.getOperand(0).getValueType();
+
+  EVT DstVT = Op.getValueType();
+
+  if (Subtarget->isVIS3()) {
+    if (DstVT == MVT::f32 && SrcVT == MVT::i32) {
+      return Op; // Legal
+    } else if (DstVT == MVT::f64 && SrcVT == MVT::i64) {
+      return (Subtarget->is64Bit())
+                 ? Op
+                 : SDValue(); // Legal on 64 bit, otherwise Expand
+    } else if (DstVT == MVT::i64 && SrcVT == MVT::f64) {
+      return (Subtarget->is64Bit())
+                 ? Op
+                 : SDValue(); // Legal on 64 bit, otherwise Expand
+    }
+  }
+
+  // Expand
+  return SDValue();
+}
+
+SDValue SparcTargetLowering::LowerUINT_TO_FP(SDValue Op,
+                                             SelectionDAG &DAG) const {
+  SDLoc dl(Op);
+  EVT OpVT = Op.getOperand(0).getValueType();
+  assert(OpVT == MVT::i32 || OpVT == MVT::i64);
+
+  // Expand f128 operations to fp128 ABI calls.
+  if (Op.getValueType() == MVT::f128 &&
+      (!Subtarget->hasHardQuad() || !isTypeLegal(OpVT))) {
+    return LowerF128Op(Op, DAG,
+                       getLibcallName(OpVT == MVT::i32
+                                          ? RTLIB::UINTTOFP_I32_F128
+                                          : RTLIB::UINTTOFP_I64_F128),
+                       1);
+  }
+
+  // Since UINT_TO_FP is legal (it's marked custom), dag combiner won't
+  // optimize it to a SINT_TO_FP when the sign bit is known zero. Perform
+  // the optimization here.
+  if (DAG.SignBitIsZero(Op.getOperand(0))) {
+
+    EVT floatVT = MVT::f32;
+    unsigned IntToFloatOpcode = SPISD::ITOF;
+
+    if (OpVT == MVT::i64) {
+      floatVT = MVT::f64;
+      IntToFloatOpcode = SPISD::XTOF;
+    }
+
+    // Convert the int value to FP in an FP register.
+    SDValue FloatTmp = DAG.getNode(ISD::BITCAST, dl, floatVT, Op.getOperand(0));
+
+    return DAG.getNode(IntToFloatOpcode, dl, Op.getValueType(), FloatTmp);
+  }
+
+  if (OpVT == MVT::i32 && Subtarget->is64Bit()) {
+
+    SDValue Int64Tmp =
+        DAG.getNode(ISD::ZERO_EXTEND, dl, MVT::i64, Op.getOperand(0));
+
+    SDValue Float64Tmp = DAG.getNode(ISD::BITCAST, dl, MVT::f64, Int64Tmp);
+
+    return DAG.getNode(SPISD::XTOF, dl, Op.getValueType(), Float64Tmp);
+  }
+
+  return SDValue();
+}
+
 //===----------------------------------------------------------------------===//
 // TargetLowering Implementation
 //===----------------------------------------------------------------------===//
 
 TargetLowering::AtomicExpansionKind SparcTargetLowering::shouldExpandAtomicRMWInIR(AtomicRMWInst *AI) const {
   if (AI->getOperation() == AtomicRMWInst::Xchg &&
       AI->getType()->getPrimitiveSizeInBits() == 32)
     return AtomicExpansionKind::None; // Uses xchg instruction
SparcTargetLowering::SparcTargetLowering(const TargetMachine &TM,
   setOperationAction(ISD::SINT_TO_FP, MVT::i64, Custom);
 
   // Custom Expand fp<->uint
   setOperationAction(ISD::FP_TO_UINT, MVT::i32, Custom);
   setOperationAction(ISD::UINT_TO_FP, MVT::i32, Custom);
   setOperationAction(ISD::FP_TO_UINT, MVT::i64, Custom);
   setOperationAction(ISD::UINT_TO_FP, MVT::i64, Custom);
 
-  setOperationAction(ISD::BITCAST, MVT::f32, Expand);
-  setOperationAction(ISD::BITCAST, MVT::i32, Expand);
-
   // Sparc has no select or setcc: expand to SELECT_CC.
   setOperationAction(ISD::SELECT, MVT::i32, Expand);
   setOperationAction(ISD::SELECT, MVT::f32, Expand);
   setOperationAction(ISD::SELECT, MVT::f64, Expand);
   setOperationAction(ISD::SELECT, MVT::f128, Expand);
 
   setOperationAction(ISD::SETCC, MVT::i32, Expand);
   setOperationAction(ISD::SETCC, MVT::f32, Expand);
SparcTargetLowering::SparcTargetLowering(const TargetMachine &TM,
   setOperationAction(ISD::SELECT_CC, MVT::i32, Custom);
   setOperationAction(ISD::SELECT_CC, MVT::f32, Custom);
   setOperationAction(ISD::SELECT_CC, MVT::f64, Custom);
   setOperationAction(ISD::SELECT_CC, MVT::f128, Custom);
 
   setOperationAction(ISD::EH_SJLJ_SETJMP, MVT::i32, Custom);
   setOperationAction(ISD::EH_SJLJ_LONGJMP, MVT::Other, Custom);
 
+  setOperationAction(ISD::BITCAST, MVT::i32, Custom);
+  setOperationAction(ISD::BITCAST, MVT::f32, Custom);
+
   if (Subtarget->is64Bit()) {
     setOperationAction(ISD::ADDC, MVT::i64, Custom);
     setOperationAction(ISD::ADDE, MVT::i64, Custom);
     setOperationAction(ISD::SUBC, MVT::i64, Custom);
     setOperationAction(ISD::SUBE, MVT::i64, Custom);
-    setOperationAction(ISD::BITCAST, MVT::f64, Expand);
-    setOperationAction(ISD::BITCAST, MVT::i64, Expand);
     setOperationAction(ISD::SELECT, MVT::i64, Expand);
     setOperationAction(ISD::SETCC, MVT::i64, Expand);
     setOperationAction(ISD::BR_CC, MVT::i64, Custom);
     setOperationAction(ISD::SELECT_CC, MVT::i64, Custom);
 
     setOperationAction(ISD::CTPOP, MVT::i64,
                        Subtarget->usePopc() ? Legal : Expand);
     setOperationAction(ISD::CTTZ , MVT::i64, Expand);
     setOperationAction(ISD::CTLZ , MVT::i64, Expand);
     setOperationAction(ISD::BSWAP, MVT::i64, Expand);
     setOperationAction(ISD::ROTL , MVT::i64, Expand);
     setOperationAction(ISD::ROTR , MVT::i64, Expand);
     setOperationAction(ISD::DYNAMIC_STACKALLOC, MVT::i64, Custom);
+
+    setOperationAction(ISD::BITCAST, MVT::i64, Custom);
+    setOperationAction(ISD::BITCAST, MVT::f64, Custom);
   }
 
   // ATOMICs.
   // Atomics are supported on SparcV9. 32-bit atomics are also
   // supported by some Leon SparcV8 variants. Otherwise, atomics
   // are unsupported.
   if (Subtarget->isV9())
     setMaxAtomicSizeInBitsSupported(64);
static SDValue LowerFP_TO_UINT(SDValue Op, SelectionDAG &DAG,
 
   return TLI.LowerF128Op(Op, DAG,
                          TLI.getLibcallName(VT == MVT::i32
                                             ? RTLIB::FPTOUINT_F128_I32
                                             : RTLIB::FPTOUINT_F128_I64),
                          1);
 }
 
-static SDValue LowerUINT_TO_FP(SDValue Op, SelectionDAG &DAG,
-                               const SparcTargetLowering &TLI,
-                               bool hasHardQuad) {
-  SDLoc dl(Op);
-  EVT OpVT = Op.getOperand(0).getValueType();
-  assert(OpVT == MVT::i32 || OpVT == MVT::i64);
-
-  // Expand if it does not involve f128 or the target has support for
-  // quad floating point instructions and the operand type is legal.
-  if (Op.getValueType() != MVT::f128 || (hasHardQuad && TLI.isTypeLegal(OpVT)))
-    return SDValue();
-
-  return TLI.LowerF128Op(Op, DAG,
-                         TLI.getLibcallName(OpVT == MVT::i32
-                                            ? RTLIB::UINTTOFP_I32_F128
-                                            : RTLIB::UINTTOFP_I64_F128),
-                         1);
-}
-
 static SDValue LowerBR_CC(SDValue Op, SelectionDAG &DAG,
                           const SparcTargetLowering &TLI,
                           bool hasHardQuad) {
   SDValue Chain = Op.getOperand(0);
   ISD::CondCode CC = cast<CondCodeSDNode>(Op.getOperand(1))->get();
   SDValue LHS = Op.getOperand(2);
   SDValue RHS = Op.getOperand(3);
   SDValue Dest = Op.getOperand(4);
LowerOperation(SDValue Op, SelectionDAG &DAG) const {
   case ISD::BlockAddress:       return LowerBlockAddress(Op, DAG);
   case ISD::ConstantPool:       return LowerConstantPool(Op, DAG);
   case ISD::FP_TO_SINT:         return LowerFP_TO_SINT(Op, DAG, *this,
                                                        hasHardQuad);
   case ISD::SINT_TO_FP:         return LowerSINT_TO_FP(Op, DAG, *this,
                                                        hasHardQuad);
   case ISD::FP_TO_UINT:         return LowerFP_TO_UINT(Op, DAG, *this,
                                                        hasHardQuad);
-  case ISD::UINT_TO_FP:         return LowerUINT_TO_FP(Op, DAG, *this,
-                                                       hasHardQuad);
+  case ISD::UINT_TO_FP:         return LowerUINT_TO_FP(Op, DAG);
   case ISD::BR_CC:              return LowerBR_CC(Op, DAG, *this,
                                                   hasHardQuad);
   case ISD::SELECT_CC:          return LowerSELECT_CC(Op, DAG, *this,
                                                       hasHardQuad);
   case ISD::EH_SJLJ_SETJMP:     return LowerEH_SJLJ_SETJMP(Op, DAG, *this);
   case ISD::EH_SJLJ_LONGJMP:    return LowerEH_SJLJ_LONGJMP(Op, DAG, *this);
   case ISD::VASTART:            return LowerVASTART(Op, DAG, *this);
   case ISD::VAARG:              return LowerVAARG(Op, DAG);
LowerOperation(SDValue Op, SelectionDAG &DAG) const {
   case ISD::ADDE:
   case ISD::SUBC:
   case ISD::SUBE:               return LowerADDC_ADDE_SUBC_SUBE(Op, DAG);
   case ISD::UMULO:
   case ISD::SMULO:              return LowerUMULO_SMULO(Op, DAG, *this);
   case ISD::ATOMIC_LOAD:
   case ISD::ATOMIC_STORE:       return LowerATOMIC_LOAD_STORE(Op, DAG);
   case ISD::INTRINSIC_WO_CHAIN: return LowerINTRINSIC_WO_CHAIN(Op, DAG);
+  case ISD::BITCAST:            return LowerBITCAST(Op, DAG);
   }
 }
 
 MachineBasicBlock *
 SparcTargetLowering::EmitInstrWithCustomInserter(MachineInstr &MI,
                                                  MachineBasicBlock *BB) const {
   switch (MI.getOpcode()) {
   default: llvm_unreachable("Unknown SELECT_CC!");

lib/Target/Sparc/SparcInstrVIS.td

 def FSLAS32   : VISInst<0b000101101, "fslas32">;
 def FSRA32    : VISInst<0b000101111, "fsra32">;
 
 let rs1 = 0 in
 def LZCNT     : VISInstFormat<0b000010111, (outs I64Regs:$rd),
                    (ins I64Regs:$rs2), "lzcnt $rs2, $rd", []>;
 
 let rs1 = 0 in {
-def MOVSTOSW : VISInstFormat<0b100010011, (outs I64Regs:$rd),
-                   (ins DFPRegs:$rs2), "movstosw $rs2, $rd", []>;
-def MOVSTOUW : VISInstFormat<0b100010001, (outs I64Regs:$rd),
-                   (ins DFPRegs:$rs2), "movstouw $rs2, $rd", []>;
-def MOVDTOX  : VISInstFormat<0b100010000, (outs I64Regs:$rd),
-                   (ins DFPRegs:$rs2), "movdtox $rs2, $rd", []>;
-def MOVWTOS  :  VISInstFormat<0b100011001, (outs DFPRegs:$rd),
-                   (ins I64Regs:$rs2), "movdtox $rs2, $rd", []>;
-def MOVXTOD  :  VISInstFormat<0b100011000, (outs DFPRegs:$rd),
-                   (ins I64Regs:$rs2), "movdtox $rs2, $rd", []>;
+def MOVSTOSW : VISInstFormat<0b100010011, (outs I64Regs:$rd), (ins FPRegs:$rs2), 
+                   "movstosw $rs2, $rd", 
+                   [(set I64Regs:$rd, (sext (i32 (bitconvert FPRegs:$rs2))))]>;
+def MOVSTOUW : VISInstFormat<0b100010001, (outs I64Regs:$rd), (ins FPRegs:$rs2), 
+                   "movstouw $rs2, $rd", 
+                   [(set I64Regs:$rd, (zext (i32 (bitconvert FPRegs:$rs2))))]>;
+def MOVDTOX  : VISInstFormat<0b100010000, (outs I64Regs:$rd), (ins DFPRegs:$rs2), 
+                   "movdtox $rs2, $rd", 
+                   [(set I64Regs:$rd, (bitconvert DFPRegs:$rs2))]>;
+def MOVWTOS  :  VISInstFormat<0b100011001, (outs FPRegs:$rd), (ins IntRegs:$rs2), 
+                   "movwtos $rs2, $rd", 
+                   [(set FPRegs:$rd, (bitconvert i32:$rs2))]>;
+def MOVXTOD  :  VISInstFormat<0b100011000, (outs DFPRegs:$rd), (ins I64Regs:$rs2),
+                   "movxtod $rs2, $rd", 
+                   [(set DFPRegs:$rd, (bitconvert I64Regs:$rs2))]>;
 }
 
 def PDISTN   : VISInst<0b000111111, "pdistn">;
 
 def UMULXHI  : VISInst<0b000010110, "umulxhi", I64Regs>;
 def XMULX    : VISInst<0b100010101, "xmulx",   I64Regs>;
 def XMULXHI  : VISInst<0b100010111, "xmulxhi", I64Regs>;
 } // Predicates = [IsVIS3]

test/CodeGen/SPARC/float.ll

 ; V8:          fdtos
 ; V8:          fstoi
 
 ; V9-LABEL:    test_utos_stou
 ; V9:          fdtos
 ; V9:          fstoi
 
 ; SPARC64-LABEL:    test_utos_stou
-; SPARC64:     fdtos
+; SPARC64:     fxtos
 ; SPARC64:     fstoi
 
 define void @test_utos_stou(i32 %a, i32* %ptr0, float* %ptr1) {
 entry:
   %0 = uitofp i32 %a to float
   store float %0, float* %ptr1, align 8
   %1 = fptoui float %0 to i32
   store i32 %1, i32* %ptr0, align 8

test/CodeGen/SPARC/uint_to_fp.ll

+
+;
+; RUN: llc -march=sparc < %s 2>&1 | FileCheck %s
+; RUN: llc -march=sparcv9 < %s 2>&1 | FileCheck --check-prefix=CHECK --check-prefix=CHECK64 %s
+
+; check that fitod used for SPISD::ITOF
+;
+; CHECK-LABEL:  Primitive_us 
+; CHECK:fitod
+
+; Function Attrs: norecurse nounwind readnone
+define double @Primitive_us(i16 zeroext) local_unnamed_addr #0 {
+  %2 = uitofp i16 %0 to double
+  ret double %2
+}
+
+; check that fxtod used for SPISD::XTOF
+;
+; CHECK-LABEL:  Primitive_ui
+; CHECK64:fxtod  
+
+; Function Attrs: norecurse nounwind readnone
+define double @Primitive_ui(i32 zeroext) local_unnamed_addr #0 {
+  %2 = uitofp i32 %0 to double
+  ret double %2
+}
+
+

test/CodeGen/SPARC/vis3.ll

+
+;
+; RUN: llc -march=sparc -mcpu=niagara4 < %s 2>&1 | FileCheck %s
+; RUN: llc -march=sparcv9 -mcpu=niagara4 < %s 2>&1 | FileCheck --check-prefix=CHECK --check-prefix=CHECK64 %s
+
+; check that movwtos used for bitcast
+;
+; CHECK-LABEL:  Primitive_us
+; CHECK:movwtos
+
+; Function Attrs: norecurse nounwind readnone
+define double @Primitive_us(i16 zeroext) local_unnamed_addr #0 {
+  %2 = uitofp i16 %0 to double
+  ret double %2
+}
+
+; check for movxtod used for bitcast 
+;
+; CHECK-LABEL:  Primitive_ui
+; CHECK64:movxtod  
+
+; Function Attrs: norecurse nounwind readnone
+define double @Primitive_ui(i32 zeroext) local_unnamed_addr #0 {
+  %2 = uitofp i32 %0 to double
+  ret double %2
+}
+
+