[PATCH 2/4] SelectionDAG: Expand SELECT_CC to SELECT + SETCC

Pete Cooper peter_cooper at apple.com
Mon May 12 15:53:22 PDT 2014


Hi Tom

Sorry for the late comments.

I can’t speak for the targets themselves but the LegalizeDAG part looks good, with just one comment...
On May 5, 2014, at 8:17 PM, Tom Stellard <thomas.stellard at amd.com> wrote:

> This consolidates code from the Hexagon, R600, and XCore targets.
> 
> No functionality change intended.
> ---
> lib/CodeGen/SelectionDAG/LegalizeDAG.cpp   | 15 +++++++++++++++
> lib/Target/Hexagon/HexagonISelLowering.cpp | 20 ++------------------
> lib/Target/Hexagon/HexagonISelLowering.h   |  1 -
> lib/Target/R600/SIISelLowering.cpp         | 22 ++++------------------
> lib/Target/R600/SIISelLowering.h           |  1 -
> lib/Target/XCore/XCoreISelLowering.cpp     | 13 +------------
> lib/Target/XCore/XCoreISelLowering.h       |  1 -
> 7 files changed, 22 insertions(+), 51 deletions(-)
> 
> diff --git a/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp b/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp
> index dac372f..745ed4a 100644
> --- a/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp
> +++ b/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp
> @@ -3899,8 +3899,23 @@ void SelectionDAGLegalize::ExpandNode(SDNode *Node) {
>     Tmp2 = Node->getOperand(1);   // RHS
>     Tmp3 = Node->getOperand(2);   // True
>     Tmp4 = Node->getOperand(3);   // False
> +    EVT VT = Node->getValueType(0);
>     SDValue CC = Node->getOperand(4);
> 
> +    // First check if SELECT_CC nodes are legal for this type
> +    if (!TLI.isOperationLegalOrCustom(ISD::SELECT_CC, VT)) {
> +      EVT CmpVT = Tmp1.getValueType();
> +      assert(TLI.isOperationLegalOrCustom(ISD::SELECT, VT) &&
> +             TLI.isOperationLegalOrCustom(ISD::SETCC, CmpVT) &&
It looks like ExpandNode could handle SETCC being illegal here.  An illegal SETCC will be further expanded to a SELET_CC.  So we may want to remove the SETCC assert if you’re happy that we won’t hit infinite legalization.  I don’t think we will, but its hard to be 100% sure.

Thanks,
Pete
> +             "ISD::SELECT and ISD::SETCC must both be legal in order to expand "
> +             "ISD::SELECT_CC");
> +      EVT CCVT = TLI.getSetCCResultType(*DAG.getContext(), CmpVT);
> +      SDValue Cond = DAG.getNode(ISD::SETCC, dl, CCVT, Tmp1, Tmp2, CC);
> +      Results.push_back(DAG.getSelect(dl, VT, Cond, Tmp3, Tmp4));
> +      break;
> +    }
> +
> +    // SELECT_CC is legal, so the condition code must not be.
>     bool Legalized = false;
>     // Try to legalize by inverting the condition.  This is for targets that
>     // might support an ordered version of a condition, but not the unordered
> diff --git a/lib/Target/Hexagon/HexagonISelLowering.cpp b/lib/Target/Hexagon/HexagonISelLowering.cpp
> index b8e5d24..aa4e9b5 100644
> --- a/lib/Target/Hexagon/HexagonISelLowering.cpp
> +++ b/lib/Target/Hexagon/HexagonISelLowering.cpp
> @@ -944,21 +944,6 @@ HexagonTargetLowering::LowerVASTART(SDValue Op, SelectionDAG &DAG) const {
> }
> 
> SDValue
> -HexagonTargetLowering::LowerSELECT_CC(SDValue Op, SelectionDAG &DAG) const {
> -  SDValue LHS = Op.getOperand(0);
> -  SDValue RHS = Op.getOperand(1);
> -  SDValue CC = Op.getOperand(4);
> -  SDValue TrueVal = Op.getOperand(2);
> -  SDValue FalseVal = Op.getOperand(3);
> -  SDLoc dl(Op);
> -  SDNode* OpNode = Op.getNode();
> -  EVT SVT = OpNode->getValueType(0);
> -
> -  SDValue Cond = DAG.getNode(ISD::SETCC, dl, MVT::i1, LHS, RHS, CC);
> -  return DAG.getNode(ISD::SELECT, dl, SVT, Cond, TrueVal, FalseVal);
> -}
> -
> -SDValue
> HexagonTargetLowering::LowerConstantPool(SDValue Op, SelectionDAG &DAG) const {
>   EVT ValTy = Op.getValueType();
>   SDLoc dl(Op);
> @@ -1341,8 +1326,8 @@ HexagonTargetLowering::HexagonTargetLowering(HexagonTargetMachine
>     setOperationAction(ISD::BSWAP, MVT::i64, Expand);
> 
>     // Lower SELECT_CC to SETCC and SELECT.
> -    setOperationAction(ISD::SELECT_CC, MVT::i32,   Custom);
> -    setOperationAction(ISD::SELECT_CC, MVT::i64,   Custom);
> +    setOperationAction(ISD::SELECT_CC, MVT::i32,   Expand);
> +    setOperationAction(ISD::SELECT_CC, MVT::i64,   Expand);
> 
>     if (QRI->Subtarget.hasV5TOps()) {
> 
> @@ -1577,7 +1562,6 @@ HexagonTargetLowering::LowerOperation(SDValue Op, SelectionDAG &DAG) const {
>     case ISD::BR_JT:              return LowerBR_JT(Op, DAG);
> 
>     case ISD::DYNAMIC_STACKALLOC: return LowerDYNAMIC_STACKALLOC(Op, DAG);
> -    case ISD::SELECT_CC:          return LowerSELECT_CC(Op, DAG);
>     case ISD::SELECT:             return Op;
>     case ISD::INTRINSIC_WO_CHAIN: return LowerINTRINSIC_WO_CHAIN(Op, DAG);
>     case ISD::INLINEASM:          return LowerINLINEASM(Op, DAG);
> diff --git a/lib/Target/Hexagon/HexagonISelLowering.h b/lib/Target/Hexagon/HexagonISelLowering.h
> index 4f27c27..0ddaf84 100644
> --- a/lib/Target/Hexagon/HexagonISelLowering.h
> +++ b/lib/Target/Hexagon/HexagonISelLowering.h
> @@ -124,7 +124,6 @@ namespace llvm {
>                             const SmallVectorImpl<SDValue> &OutVals,
>                             SDValue Callee) const;
> 
> -    SDValue LowerSELECT_CC(SDValue Op, SelectionDAG &DAG) const;
>     SDValue LowerFRAMEADDR(SDValue Op, SelectionDAG &DAG) const;
>     SDValue LowerATOMIC_FENCE(SDValue Op, SelectionDAG& DAG) const;
>     SDValue LowerRETURNADDR(SDValue Op, SelectionDAG &DAG) const;
> diff --git a/lib/Target/R600/SIISelLowering.cpp b/lib/Target/R600/SIISelLowering.cpp
> index cacff83..57ac274 100644
> --- a/lib/Target/R600/SIISelLowering.cpp
> +++ b/lib/Target/R600/SIISelLowering.cpp
> @@ -103,10 +103,10 @@ SITargetLowering::SITargetLowering(TargetMachine &TM) :
>   setOperationAction(ISD::SELECT, MVT::f64, Promote);
>   AddPromotedToType(ISD::SELECT, MVT::f64, MVT::i64);
> 
> -  setOperationAction(ISD::SELECT_CC, MVT::f32, Custom);
> -  setOperationAction(ISD::SELECT_CC, MVT::i32, Custom);
> -
> -  setOperationAction(ISD::SELECT_CC, MVT::Other, Expand);
> +  setOperationAction(ISD::SELECT_CC, MVT::f32, Expand);
> +  setOperationAction(ISD::SELECT_CC, MVT::i32, Expand);
> +  setOperationAction(ISD::SELECT_CC, MVT::i64, Expand);
> +  setOperationAction(ISD::SELECT_CC, MVT::f64, Expand);
> 
>   setOperationAction(ISD::SETCC, MVT::v2i1, Expand);
>   setOperationAction(ISD::SETCC, MVT::v4i1, Expand);
> @@ -601,7 +601,6 @@ SDValue SITargetLowering::LowerOperation(SDValue Op, SelectionDAG &DAG) const {
>   }
> 
>   case ISD::SELECT: return LowerSELECT(Op, DAG);
> -  case ISD::SELECT_CC: return LowerSELECT_CC(Op, DAG);
>   case ISD::SIGN_EXTEND: return LowerSIGN_EXTEND(Op, DAG);
>   case ISD::STORE: return LowerSTORE(Op, DAG);
>   case ISD::ANY_EXTEND: // Fall-through
> @@ -893,19 +892,6 @@ SDValue SITargetLowering::LowerSELECT(SDValue Op, SelectionDAG &DAG) const {
>   return DAG.getNode(ISD::BITCAST, DL, MVT::i64, Res);
> }
> 
> -SDValue SITargetLowering::LowerSELECT_CC(SDValue Op, SelectionDAG &DAG) const {
> -  SDValue LHS = Op.getOperand(0);
> -  SDValue RHS = Op.getOperand(1);
> -  SDValue True = Op.getOperand(2);
> -  SDValue False = Op.getOperand(3);
> -  SDValue CC = Op.getOperand(4);
> -  EVT VT = Op.getValueType();
> -  SDLoc DL(Op);
> -
> -  SDValue Cond = DAG.getNode(ISD::SETCC, DL, MVT::i1, LHS, RHS, CC);
> -  return DAG.getNode(ISD::SELECT, DL, VT, Cond, True, False);
> -}
> -
> SDValue SITargetLowering::LowerSIGN_EXTEND(SDValue Op,
>                                            SelectionDAG &DAG) const {
>   EVT VT = Op.getValueType();
> diff --git a/lib/Target/R600/SIISelLowering.h b/lib/Target/R600/SIISelLowering.h
> index c6eaa81..3253f21 100644
> --- a/lib/Target/R600/SIISelLowering.h
> +++ b/lib/Target/R600/SIISelLowering.h
> @@ -27,7 +27,6 @@ class SITargetLowering : public AMDGPUTargetLowering {
>                                SelectionDAG &DAG) const;
>   SDValue LowerLOAD(SDValue Op, SelectionDAG &DAG) const;
>   SDValue LowerSELECT(SDValue Op, SelectionDAG &DAG) const;
> -  SDValue LowerSELECT_CC(SDValue Op, SelectionDAG &DAG) const;
>   SDValue LowerSIGN_EXTEND(SDValue Op, SelectionDAG &DAG) const;
>   SDValue LowerSTORE(SDValue Op, SelectionDAG &DAG) const;
>   SDValue LowerZERO_EXTEND(SDValue Op, SelectionDAG &DAG) const;
> diff --git a/lib/Target/XCore/XCoreISelLowering.cpp b/lib/Target/XCore/XCoreISelLowering.cpp
> index 1c52c70..96eebb4 100644
> --- a/lib/Target/XCore/XCoreISelLowering.cpp
> +++ b/lib/Target/XCore/XCoreISelLowering.cpp
> @@ -92,7 +92,7 @@ XCoreTargetLowering::XCoreTargetLowering(XCoreTargetMachine &XTM)
> 
>   // XCore does not have the NodeTypes below.
>   setOperationAction(ISD::BR_CC,     MVT::i32,   Expand);
> -  setOperationAction(ISD::SELECT_CC, MVT::i32,   Custom);
> +  setOperationAction(ISD::SELECT_CC, MVT::i32,   Expand);
>   setOperationAction(ISD::ADDC, MVT::i32, Expand);
>   setOperationAction(ISD::ADDE, MVT::i32, Expand);
>   setOperationAction(ISD::SUBC, MVT::i32, Expand);
> @@ -217,7 +217,6 @@ LowerOperation(SDValue Op, SelectionDAG &DAG) const {
>   case ISD::BR_JT:              return LowerBR_JT(Op, DAG);
>   case ISD::LOAD:               return LowerLOAD(Op, DAG);
>   case ISD::STORE:              return LowerSTORE(Op, DAG);
> -  case ISD::SELECT_CC:          return LowerSELECT_CC(Op, DAG);
>   case ISD::VAARG:              return LowerVAARG(Op, DAG);
>   case ISD::VASTART:            return LowerVASTART(Op, DAG);
>   case ISD::SMUL_LOHI:          return LowerSMUL_LOHI(Op, DAG);
> @@ -259,16 +258,6 @@ void XCoreTargetLowering::ReplaceNodeResults(SDNode *N,
> //===----------------------------------------------------------------------===//
> 
> SDValue XCoreTargetLowering::
> -LowerSELECT_CC(SDValue Op, SelectionDAG &DAG) const
> -{
> -  SDLoc dl(Op);
> -  SDValue Cond = DAG.getNode(ISD::SETCC, dl, MVT::i32, Op.getOperand(2),
> -                             Op.getOperand(3), Op.getOperand(4));
> -  return DAG.getNode(ISD::SELECT, dl, MVT::i32, Cond, Op.getOperand(0),
> -                     Op.getOperand(1));
> -}
> -
> -SDValue XCoreTargetLowering::
> getGlobalAddressWrapper(SDValue GA, const GlobalValue *GV,
>                         SelectionDAG &DAG) const
> {
> diff --git a/lib/Target/XCore/XCoreISelLowering.h b/lib/Target/XCore/XCoreISelLowering.h
> index 4e662fc..fc8b364 100644
> --- a/lib/Target/XCore/XCoreISelLowering.h
> +++ b/lib/Target/XCore/XCoreISelLowering.h
> @@ -157,7 +157,6 @@ namespace llvm {
>     SDValue LowerBlockAddress(SDValue Op, SelectionDAG &DAG) const;
>     SDValue LowerConstantPool(SDValue Op, SelectionDAG &DAG) const;
>     SDValue LowerBR_JT(SDValue Op, SelectionDAG &DAG) const;
> -    SDValue LowerSELECT_CC(SDValue Op, SelectionDAG &DAG) const;
>     SDValue LowerVAARG(SDValue Op, SelectionDAG &DAG) const;
>     SDValue LowerVASTART(SDValue Op, SelectionDAG &DAG) const;
>     SDValue LowerUMUL_LOHI(SDValue Op, SelectionDAG &DAG) const;
> -- 
> 1.8.1.5
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits





More information about the llvm-commits mailing list