[Power9] Add __float128 support in the backend for bitcast to a i128

[stefanp]

Summary

Add support to allow bit-casting from f128 to i128 and then extracting 64 bits from the result.

[lei]

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

14129 ↗

(On Diff #157346)

Can you add some general documentation for the function below to describe what it does?

[nemanjai]

This is definitely going to need a test case.

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

14137 ↗

(On Diff #157346)

Why the call to getNode()? An SDValue is implicitly convertible to bool and will be false if this is a default-constructed one.

[stefanp]

Added test case.
Address reviewers comments.

[nemanjai]

lib/Target/PowerPC/PPCISelLowering.cpp

14192

Nit: indentation is off.

14195

Perhaps for brevity and readability, combine this into the if:

if (SDValue CRTruncValue = DAGCombineTruncBoolExt(N, DCI))
  return CRTruncValue;

14203

Nit: the more common way to declare references puts the ampersand immediately before the variable name.

14209

Why not just Op0.getOpcode() and Op0.getOperand(0)? Here as well as below.

lib/Target/PowerPC/PPCInstrInfo.td

237

I don't think it is OK to allow arbitrary integer types for the result nor arbitrary floating point types for operand 1. This really seems like it should use SDTCisVT. I realize that PPCbuild_fp128 has the same issue and we missed it, but I think we should favour fixing that one (in a separate patch) rather than making this one match.

test/CodeGen/PowerPC/f128-bitcast.ll

1

I think once you add the BE portion to this patch, we should have another RUN to test that.

[nemanjai]

Since there are unaddressed comments, I'm moving this off of my Must Review queue until they are addressed.

[stefanp]

Address Review Comments

[efriedma]

Instead of generating a special EXTRACT_FP128 node, could you bitcast to v2i64 and use EXTRACT_VECTOR_ELT instead? (I'm not that familiar with Power9, so it's possible that doesn't actually work for some reason.)

[stefanp]

Removed the custom PPCISD node and it simplified the patch considerably.

Added conditions to handle bitconverts from f128 to a number of vector sizes (v2i64 down to v16i8).
Did not add the code for the bitconvert from the vector sizes to f128 because I have not been able to find an example to trigger the condition. It seems that the f128 is lowered into a store followed by a load.

[efriedma]

lib/Target/PowerPC/PPCISelLowering.cpp

14212

80 columns.

I assume you want something like (uint32_t)DAG.getDataLayout().isBigEndian() here, not "0".

14218

The second operand of the shift might not be a constant; getConstantOperandVal will crash in that case.

[stefanp]

Addressed review comments.

Also added Big Endian to the test to cover that case.

[nemanjai]

lib/Target/PowerPC/PPCISelLowering.cpp

14202

This logic is fairly deeply nested. Can you convert some of the conditions to early exits. This one certainly looks to fit the bill.

14219

For clarity, please do the null pointer check first.

[stefanp]

Simplified some of the conditionals (easier to read) and added an early exit.

[nemanjai]

lib/Target/PowerPC/PPCISelLowering.cpp

14219

I think this can be further simplified and the duplicated code can be removed by essentially "looking through" the right shift and updating the element you need to extract.

[stefanp]

Simplified the code further.

[nemanjai]

I think the changes that are needed are clear enough that this doesn't require another review cycle. Approving this with the assumption that the required change will be made on the commit.

lib/Target/PowerPC/PPCISelLowering.cpp

14213

This seems kind of misleading - makes the reader wonder what this "constant bit" is. I think something like // Switch the element number to extract. is a lot more descriptive.

14217

There needs to be an else here that will return SDValue() since it is not OK to keep going if we have an SRL with a non-constant shift amount or a shift amount other than 64.
However, rather than an else I think it would be nicer to flip the condition and exit early...

// The right shift has to be by 64 bits.
if (!ConstNode || ConstNode->getZextValue() != 64)
  return SDValue();

14224

The comment is redundant - basically just says that we're inside the if block.

lib/Target/PowerPC/P9InstrResources.td

def : InstRW<[P9_PM_3C, IP_EXECO_1C, IP_EXECE_1C, DISP_1C, DISP_1C, DISP_1C],
       (instrs
     (instregex "LVS(L|R)$"),
     (instregex "VSPLTIS(W|H|B)$"),
     (instregex "VSPLT(W|H|B)(s)?$"),
     (instregex "V_SETALLONES(B|H)?$"),
     (instregex "VEXTRACTU(B|H|W)$"),
     (instregex "VINSERT(B|H|W|D)$"),
     MFVSRLD,
+    MFVRLD,
     MTVSRWS,
     VBPERMQ,
     VCLZLSBB,
     VCTZLSBB,
     VEXTRACTD,
     VEXTUBLX,
     VEXTUBRX,
     VEXTUHLX,

lib/Target/PowerPC/PPCISelLowering.h

enum NodeType : unsigned {
       MTVSRA,
 
       /// Direct move from a GPR to a VSX register (zero)
       MTVSRZ,
 
       /// Direct move of 2 consective GPR to a VSX register.
       BUILD_FP128,
 
+      /// Extract bits from a float128
+      EXTRACT_FP128,
+
       /// Extract a subvector from signed integer vector and convert to FP.
       /// It is primarily used to convert a (widened) illegal integer vector
       /// type to a legal floating point vector type.
       /// For example v2i32 -> widened to v4i32 -> v2f64
       SINT_VEC_TO_FP,
 
       /// Extract a subvector from unsigned integer vector and convert to FP.
       /// As with SINT_VEC_TO_FP, used for converting illegal types.
private:
     SDValue DAGCombineExtBoolTrunc(SDNode *N, DAGCombinerInfo &DCI) const;
     SDValue DAGCombineBuildVector(SDNode *N, DAGCombinerInfo &DCI) const;
     SDValue DAGCombineTruncBoolExt(SDNode *N, DAGCombinerInfo &DCI) const;
     SDValue combineStoreFPToInt(SDNode *N, DAGCombinerInfo &DCI) const;
     SDValue combineFPToIntToFP(SDNode *N, DAGCombinerInfo &DCI) const;
     SDValue combineSHL(SDNode *N, DAGCombinerInfo &DCI) const;
     SDValue combineSRA(SDNode *N, DAGCombinerInfo &DCI) const;
     SDValue combineSRL(SDNode *N, DAGCombinerInfo &DCI) const;
+    SDValue combineTRUNCATE(SDNode *N, DAGCombinerInfo &DCI) const;
 
     /// ConvertSETCCToSubtract - looks at SETCC that compares ints. It replaces
     /// SETCC with integer subtraction when (1) there is a legal way of doing it
     /// (2) keeping the result of comparison in GPR has performance benefit.
     SDValue ConvertSETCCToSubtract(SDNode *N, DAGCombinerInfo &DCI) const;
 
     SDValue getSqrtEstimate(SDValue Operand, SelectionDAG &DAG, int Enabled,
                             int &RefinementSteps, bool &UseOneConstNR,

lib/Target/PowerPC/PPCISelLowering.cpp

PPCTargetLowering::PPCTargetLowering(const PPCTargetMachine &TM,
   setTargetDAGCombine(ISD::INTRINSIC_WO_CHAIN);
   setTargetDAGCombine(ISD::INTRINSIC_W_CHAIN);
   setTargetDAGCombine(ISD::INTRINSIC_VOID);
 
   setTargetDAGCombine(ISD::SIGN_EXTEND);
   setTargetDAGCombine(ISD::ZERO_EXTEND);
   setTargetDAGCombine(ISD::ANY_EXTEND);
 
+  setTargetDAGCombine(ISD::TRUNCATE);
+
   if (Subtarget.useCRBits()) {
     setTargetDAGCombine(ISD::TRUNCATE);
     setTargetDAGCombine(ISD::SETCC);
     setTargetDAGCombine(ISD::SELECT_CC);
   }
 
   // Use reciprocal estimates.
   if (TM.Options.UnsafeFPMath) {
const char *PPCTargetLowering::getTargetNodeName(unsigned Opcode) const {
   case PPCISD::SWAP_NO_CHAIN:   return "PPCISD::SWAP_NO_CHAIN";
   case PPCISD::QVFPERM:         return "PPCISD::QVFPERM";
   case PPCISD::QVGPCI:          return "PPCISD::QVGPCI";
   case PPCISD::QVALIGNI:        return "PPCISD::QVALIGNI";
   case PPCISD::QVESPLATI:       return "PPCISD::QVESPLATI";
   case PPCISD::QBFLT:           return "PPCISD::QBFLT";
   case PPCISD::QVLFSb:          return "PPCISD::QVLFSb";
   case PPCISD::BUILD_FP128:     return "PPCISD::BUILD_FP128";
+  case PPCISD::EXTRACT_FP128:   return "PPCISD::EXTRACT_FP128";
   case PPCISD::EXTSWSLI:        return "PPCISD::EXTSWSLI";
   }
   return nullptr;
 }
 
 EVT PPCTargetLowering::getSetCCResultType(const DataLayout &DL, LLVMContext &C,
                                           EVT VT) const {
   if (!VT.isVector())
void PPCTargetLowering::ReplaceNodeResults(SDNode *N,
   }
   case ISD::FP_TO_SINT:
   case ISD::FP_TO_UINT:
     // LowerFP_TO_INT() can only handle f32 and f64.
     if (N->getOperand(0).getValueType() == MVT::ppcf128)
       return;
     Results.push_back(LowerFP_TO_INT(SDValue(N, 0), DAG, dl));
     return;
+  case ISD::BITCAST:
+    // Don't handle bitcast here.
+    return;
   }
 }
 
 //===----------------------------------------------------------------------===//
 //  Other Lowering Code
 //===----------------------------------------------------------------------===//
 
 static Instruction* callIntrinsic(IRBuilder<> &Builder, Intrinsic::ID Id) {
if (ConstantSDNode *C = dyn_cast<ConstantSDNode>(N->getOperand(0))) {
         return N->getOperand(0);
     }
     break;
   case ISD::SIGN_EXTEND:
   case ISD::ZERO_EXTEND:
   case ISD::ANY_EXTEND:
     return DAGCombineExtBoolTrunc(N, DCI);
   case ISD::TRUNCATE:
+    return combineTRUNCATE(N, DCI);
   case ISD::SETCC:
   case ISD::SELECT_CC:
     return DAGCombineTruncBoolExt(N, DCI);
   case ISD::SINT_TO_FP:
   case ISD::UINT_TO_FP:
     return combineFPToIntToFP(N, DCI);
   case ISD::STORE: {
 

 
 SDValue PPCTargetLowering::combineSRL(SDNode *N, DAGCombinerInfo &DCI) const {
   if (auto Value = stripModuloOnShift(*this, N, DCI.DAG))
     return Value;
 
   return SDValue();
 }
 
+// Detect TRUNCATE operations on bitcasts of float128 values.
+// What we are looking for here is the situtation where we extract a subset
+// of bits from a 128 bit float.
+// This can be of two forms:
+// 1) BITCAST of f128 feeding TRUNCATE
+// 2) BITCAST of f128 feeding SRL (a shift) feeding TRUNCATE
+// The reason this is required is because we do not have a legal i128 type
+// and so we want to prevent having to store the f128 and then reload part
+// of it.
+SDValue PPCTargetLowering::combineTRUNCATE(SDNode *N,
+                                            DAGCombinerInfo &DCI) const {
+  // If we are using CRBits then try that first.
+  if (Subtarget.useCRBits()) {
+    SDValue CRTruncValue = DAGCombineTruncBoolExt(N, DCI);
+
+    // Check if CRBits did anything and return that if it did.
+    if (CRTruncValue)
+      return CRTruncValue;
+  }
+
+  SDLoc dl(N);
+  const SDValue & Op0 = N->getOperand(0);
+  if (Op0.getValueType() == MVT::i128 &&
+      N->getValueType(0) == MVT::i64) {
+
+    EVT TCVT = MVT::i32;
+    // BITCAST feeding a TRUNCATE
+    if (Op0.getNode()->getOpcode() == ISD::BITCAST &&
+        Op0.getNode()->getOperand(0).getValueType() == MVT::f128) {
+
+      // Have a truncate fed by a bitcast of an f128.
+      return DCI.DAG.getNode(PPCISD::EXTRACT_FP128, dl, MVT::i64,
+                            Op0.getNode()->getOperand(0),
+                            DCI.DAG.getTargetConstant(0, dl, TCVT));
+    }
+
+    // BITCAST feeding SRL feeding TRUNCATE
+    if (Op0.getNode()->getOpcode() == ISD::SRL &&
+        Op0.getNode()->getOperand(0).getValueType() == MVT::i128 &&
+        Op0.getNode()->getConstantOperandVal(1) == 64 &&
+        Op0.getNode()->getOperand(0).getNode()->getOpcode() == ISD::BITCAST) {
+      SDNode* BitcastNode = Op0.getNode()->getOperand(0).getNode();
+
+      // Check if we are bitcasting from an f128
+      if (BitcastNode->getOperand(0).getValueType() == MVT::f128) {
+        return DCI.DAG.getNode(PPCISD::EXTRACT_FP128, dl, MVT::i64,
+                              BitcastNode->getOperand(0),
+                              DCI.DAG.getTargetConstant(64, dl, TCVT));
+      }
+    }
+
+  }
+
+  return SDValue();
+}
+
 bool PPCTargetLowering::mayBeEmittedAsTailCall(const CallInst *CI) const {
   // Only duplicate to increase tail-calls for the 64bit SysV ABIs.
   if (!Subtarget.isSVR4ABI() || !Subtarget.isPPC64())
     return false;
 
   // If not a tail call then no need to proceed.
   if (!CI->isTailCall())
     return false;

lib/Target/PowerPC/PPCInstrInfo.td

 
 // Move 2 i64 values into a VSX register
 def PPCbuild_fp128: SDNode<"PPCISD::BUILD_FP128",
                            SDTypeProfile<1, 2,
                              [SDTCisFP<0>, SDTCisSameSizeAs<1,2>,
                               SDTCisSameAs<1,2>]>,
                            []>;
 
+// Extract bits from a float128
+def PPCextract_fp128: SDNode<"PPCISD::EXTRACT_FP128",
+                             SDTypeProfile<1, 2,
+                               [SDTCisInt<0>, SDTCisFP<1>, SDTCisPtrTy<2>]>,
+                             []>;
+
 // These are target-independent nodes, but have target-specific formats.
 def callseq_start : SDNode<"ISD::CALLSEQ_START", SDT_PPCCallSeqStart,
                            [SDNPHasChain, SDNPOutGlue]>;
 def callseq_end   : SDNode<"ISD::CALLSEQ_END",   SDT_PPCCallSeqEnd,
                            [SDNPHasChain, SDNPOptInGlue, SDNPOutGlue]>;
 
 def SDT_PPCCall   : SDTypeProfile<0, -1, [SDTCisInt<0>]>;
 def PPCcall  : SDNode<"PPCISD::CALL", SDT_PPCCall,

lib/Target/PowerPC/PPCInstrVSX.td

let Predicates = [IsISA3_0, HasDirectMove] in {
 
   def MTVSRDD: XX1Form<31, 435, (outs vsrc:$XT), (ins g8rc_nox0:$rA, g8rc:$rB),
                        "mtvsrdd $XT, $rA, $rB", IIC_VecGeneral,
                        []>, Requires<[In64BitMode]>;
 
   def MFVSRLD: XX1_RS6_RD5_XO<31, 307, (outs g8rc:$rA), (ins vsrc:$XT),
                               "mfvsrld $rA, $XT", IIC_VecGeneral,
                               []>, Requires<[In64BitMode]>;
+  let isCodeGenOnly = 1 in
+  def MFVRLD: XX1_RS6_RD5_XO<31, 307, (outs g8rc:$rA), (ins vrrc:$XT),
+                              "mfvsrld $rA, $XT", IIC_VecGeneral,
+                              []>, Requires<[In64BitMode]>;
 
 } // IsISA3_0, HasDirectMove
 } // UseVSXReg = 1
 
 // We want to parse this from asm, but we don't want to emit this as it would
 // be emitted with a VSX reg. So leave Emit = 0 here.
 def : InstAlias<"mfvrd $rA, $XT",
                 (MFVRD g8rc:$rA, vrrc:$XT), 0>;
def : Pat<(f128 (PPCbuild_fp128 i64:$rB, i64:$rA)),
               (f128 (COPY_TO_REGCLASS (MTVSRDD $rB, $rA), VRRC))>;
   }
   let Predicates = [IsISA3_0, HasP9Vector, HasDirectMove, IsLittleEndian] in {
     def : Pat<(f128 (PPCbuild_fp128 i64:$rA, i64:$rB)),
               (f128 (COPY_TO_REGCLASS (MTVSRDD $rB, $rA), VRRC))>;
   }
 }
 
+let Predicates = [IsISA3_0, HasP9Vector, HasDirectMove, IsLittleEndian] in {
+  def : Pat<(i64 (PPCextract_fp128 f128:$rA, 0)),
+            (i64 (MFVRD $rA))>;
+  def : Pat<(i64 (PPCextract_fp128 f128:$rA, 64)),
+            (i64 (MFVRLD $rA))>;
+}
+
 let Predicates = [HasP9Vector] in {
   let isPseudo = 1 in {
     let mayStore = 1 in {
       def SPILLTOVSR_STX : PseudoXFormMemOp<(outs),
                                             (ins spilltovsrrc:$XT, memrr:$dst),
                                             "#SPILLTOVSR_STX", []>;
       def SPILLTOVSR_ST : Pseudo<(outs), (ins spilltovsrrc:$XT, memrix:$dst),
                                 "#SPILLTOVSR_ST", []>;

test/CodeGen/PowerPC/f128-bitcast.ll

+; RUN: llc -mcpu=pwr9 -mtriple=powerpc64le-unknown-unknown \
+; RUN:   -enable-ppc-quad-precision -verify-machineinstrs \
+; RUN:   -ppc-asm-full-reg-names -ppc-vsr-nums-as-vr < %s | FileCheck %s
+
+; Function Attrs: norecurse nounwind readnone
+define i64 @getPart1(fp128 %in) local_unnamed_addr {
+entry:
+  %0 = bitcast fp128 %in to i128
+  %a.sroa.0.0.extract.trunc = trunc i128 %0 to i64
+  ret i64 %a.sroa.0.0.extract.trunc
+; CHECK-LABEL: getPart1
+; CHECK:       mfvsrd r3, v2
+; CHECK-NEXT:  blr
+}
+
+; Function Attrs: norecurse nounwind readnone
+define i64 @getPart2(fp128 %in) local_unnamed_addr {
+entry:
+  %0 = bitcast fp128 %in to i128
+  %a.sroa.0.8.extract.shift = lshr i128 %0, 64
+  %a.sroa.0.8.extract.trunc = trunc i128 %a.sroa.0.8.extract.shift to i64
+  ret i64 %a.sroa.0.8.extract.trunc
+; CHECK-LABEL: getPart2
+; CHECK:       mfvsrld r3, v2
+; CHECK-NEXT:  blr
+}
+