PATCH: SelectionDAG: Remove setOperationAction(ISD::SELECT_CC, MVT::Other, Expand) hack.
Tom Stellard
tom at stellard.net
Mon May 5 11:29:30 PDT 2014
Hi,
I've updated this patches series to fix a few more bugs that I've found, and
I've also updated the broken MIPS test case.
The first two patches which add support for expanding SELECT_CC nodes I sent
to the list in separate emails, because they are isolated from the MVT::Other
fix.
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20140505/215830.html
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20140505/215829.html
The two remaining patches which remove the ISD::SELECT_CC + MVT::Other hack are
attached to this email.
Thanks,
Tom
On Fri, May 02, 2014 at 02:35:53PM -0700, Tom Stellard wrote:
> Hi,
>
> I've always had a lot of trouble with ISD::SELECT_CC nodes in the
> SelectionDAG, and I think part of the problem is that there is a special
> case for ISD::SELECT_CC where targets can use the MVT::Other type to
> specify one action for all types. The attached patches remove this
> special case and will hopefully help to simplify the legalization and
> optimization of ISD::SELECT_CC.
>
> There is one regression on MIPS with these patches:
>
> CodeGen/Mips/selectcc.ll
>
> I can't tell if this is a real regression or just a change in the order of the
> code. Daniel, could you take a look at this. I have attached the before and
> after output to this email.
>
> Thanks,
> Tom
> .text
> .abicalls
> .section .mdebug.abi32,"", at progbits
> .nan legacy
> .file "<stdin>"
> .text
> .globl select_cc_f32
> .align 2
> .type select_cc_f32, at function
> .set nomicromips
> .set nomips16
> .ent select_cc_f32
> select_cc_f32: # @select_cc_f32
> .frame $sp,0,$ra
> .mask 0x00000000,0
> .fmask 0x00000000,0
> .set noreorder
> .set nomacro
> .set noat
> # BB#0: # %entry
> lui $2, %hi(_gp_disp)
> addiu $2, $2, %lo(_gp_disp)
> addu $1, $2, $25
> lw $2, %got(gf0)($1)
> sw $zero, 0($2)
> lw $1, %got(gf1)($1)
> lui $2, 16256
> sw $2, 0($1)
> addiu $1, $zero, 0
> addiu $2, $zero, 1
> c.olt.s $f12, $f14
> movt $1, $2, $fcc0
> mtc1 $1, $f0
> jr $ra
> cvt.s.w $f0, $f0
> .set at
> .set macro
> .set reorder
> .end select_cc_f32
> $tmp0:
> .size select_cc_f32, ($tmp0)-select_cc_f32
>
> .globl select_cc_f64
> .align 2
> .type select_cc_f64, at function
> .set nomicromips
> .set nomips16
> .ent select_cc_f64
> select_cc_f64: # @select_cc_f64
> .frame $sp,0,$ra
> .mask 0x00000000,0
> .fmask 0x00000000,0
> .set noreorder
> .set nomacro
> .set noat
> # BB#0: # %entry
> lui $2, %hi(_gp_disp)
> addiu $2, $2, %lo(_gp_disp)
> addu $1, $2, $25
> lw $2, %got(gd0)($1)
> sw $zero, 4($2)
> sw $zero, 0($2)
> lw $1, %got(gd1)($1)
> lui $2, 16368
> sw $2, 4($1)
> sw $zero, 0($1)
> addiu $1, $zero, 0
> addiu $2, $zero, 1
> c.olt.d $f12, $f14
> movt $1, $2, $fcc0
> mtc1 $1, $f0
> jr $ra
> cvt.d.w $f0, $f0
> .set at
> .set macro
> .set reorder
> .end select_cc_f64
> $tmp1:
> .size select_cc_f64, ($tmp1)-select_cc_f64
>
>
> 0x668ef0: f32 = sint_to_fp 0x6672c0 [ORD=6]
> VT = f32
> Other = 1
> VT = 1
> Action = 3
> VT Legal = 1
> Folding
> 0x666768: f64 = sint_to_fp 0x66a5a8 [ORD=6]
> VT = f64
> Other = 1
> VT = 1
> Action = 3
> VT Legal = 1
> Folding
> .text
> .abicalls
> .section .mdebug.abi32,"", at progbits
> .nan legacy
> .file "<stdin>"
> .section .rodata.cst4,"aM", at progbits,4
> .align 2
> $CPI0_0:
> .4byte 1065353216 # float 1
> .text
> .globl select_cc_f32
> .align 2
> .type select_cc_f32, at function
> .set nomicromips
> .set nomips16
> .ent select_cc_f32
> select_cc_f32: # @select_cc_f32
> .frame $sp,0,$ra
> .mask 0x00000000,0
> .fmask 0x00000000,0
> .set noreorder
> .set nomacro
> .set noat
> # BB#0: # %entry
> lui $2, %hi(_gp_disp)
> addiu $2, $2, %lo(_gp_disp)
> addu $1, $2, $25
> lw $2, %got(gf0)($1)
> sw $zero, 0($2)
> lw $2, %got(gf1)($1)
> lui $3, 16256
> sw $3, 0($2)
> lw $1, %got($CPI0_0)($1)
> lwc1 $f1, %lo($CPI0_0)($1)
> mtc1 $zero, $f0
> c.olt.s $f12, $f14
> jr $ra
> movt.s $f0, $f1, $fcc0
> .set at
> .set macro
> .set reorder
> .end select_cc_f32
> $tmp0:
> .size select_cc_f32, ($tmp0)-select_cc_f32
>
> .section .rodata.cst8,"aM", at progbits,8
> .align 3
> $CPI1_0:
> .8byte 4607182418800017408 # double 1
> .text
> .globl select_cc_f64
> .align 2
> .type select_cc_f64, at function
> .set nomicromips
> .set nomips16
> .ent select_cc_f64
> select_cc_f64: # @select_cc_f64
> .frame $sp,0,$ra
> .mask 0x00000000,0
> .fmask 0x00000000,0
> .set noreorder
> .set nomacro
> .set noat
> # BB#0: # %entry
> lui $2, %hi(_gp_disp)
> addiu $2, $2, %lo(_gp_disp)
> addu $1, $2, $25
> lw $2, %got(gd0)($1)
> sw $zero, 4($2)
> sw $zero, 0($2)
> lw $2, %got(gd1)($1)
> lui $3, 16368
> sw $3, 4($2)
> sw $zero, 0($2)
> mtc1 $zero, $f0
> mtc1 $zero, $f1
> lw $1, %got($CPI1_0)($1)
> ldc1 $f2, %lo($CPI1_0)($1)
> c.olt.d $f12, $f14
> jr $ra
> movt.d $f0, $f2, $fcc0
> .set at
> .set macro
> .set reorder
> .end select_cc_f64
> $tmp1:
> .size select_cc_f64, ($tmp1)-select_cc_f64
>
>
> From f8faa322e774fac997d54c15678abe090e146b2b Mon Sep 17 00:00:00 2001
> From: Tom Stellard <thomas.stellard at amd.com>
> Date: Fri, 2 May 2014 22:48:47 -0400
> Subject: [PATCH 1/4] R600: Move MIN/MAX matching from LowerOperation() to
> PerformDAGCombine()
>
> ---
> lib/Target/R600/AMDGPUISelLowering.cpp | 22 +++++++++++++---------
> lib/Target/R600/AMDGPUISelLowering.h | 2 +-
> lib/Target/R600/R600ISelLowering.cpp | 12 +++++-------
> lib/Target/R600/SIISelLowering.cpp | 6 ------
> test/CodeGen/R600/pv.ll | 2 +-
> 5 files changed, 20 insertions(+), 24 deletions(-)
>
> diff --git a/lib/Target/R600/AMDGPUISelLowering.cpp b/lib/Target/R600/AMDGPUISelLowering.cpp
> index 52a500c..b7e959d 100644
> --- a/lib/Target/R600/AMDGPUISelLowering.cpp
> +++ b/lib/Target/R600/AMDGPUISelLowering.cpp
> @@ -256,6 +256,7 @@ AMDGPUTargetLowering::AMDGPUTargetLowering(TargetMachine &TM) :
> }
>
> setTargetDAGCombine(ISD::MUL);
> + setTargetDAGCombine(ISD::SELECT_CC);
> }
>
> //===----------------------------------------------------------------------===//
> @@ -737,16 +738,16 @@ SDValue AMDGPUTargetLowering::LowerIntrinsicLRP(SDValue Op,
> }
>
> /// \brief Generate Min/Max node
> -SDValue AMDGPUTargetLowering::LowerMinMax(SDValue Op,
> +SDValue AMDGPUTargetLowering::CombineMinMax(SDNode *N,
> SelectionDAG &DAG) const {
> - SDLoc DL(Op);
> - EVT VT = Op.getValueType();
> + SDLoc DL(N);
> + EVT VT = N->getValueType(0);
>
> - 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);
> + SDValue LHS = N->getOperand(0);
> + SDValue RHS = N->getOperand(1);
> + SDValue True = N->getOperand(2);
> + SDValue False = N->getOperand(3);
> + SDValue CC = N->getOperand(4);
>
> if (VT != MVT::f32 ||
> !((LHS == True && RHS == False) || (LHS == False && RHS == True))) {
> @@ -793,7 +794,7 @@ SDValue AMDGPUTargetLowering::LowerMinMax(SDValue Op,
> case ISD::SETCC_INVALID:
> llvm_unreachable("Invalid setcc condcode!");
> }
> - return Op;
> + return SDValue();
> }
>
> SDValue AMDGPUTargetLowering::SplitVectorLoad(const SDValue &Op,
> @@ -1272,6 +1273,9 @@ SDValue AMDGPUTargetLowering::PerformDAGCombine(SDNode *N,
> simplifyI24(N1, DCI);
> return SDValue();
> }
> + case ISD::SELECT_CC: {
> + return CombineMinMax(N, DAG);
> + }
> }
> return SDValue();
> }
> diff --git a/lib/Target/R600/AMDGPUISelLowering.h b/lib/Target/R600/AMDGPUISelLowering.h
> index 8db476c..4a2dad3 100644
> --- a/lib/Target/R600/AMDGPUISelLowering.h
> +++ b/lib/Target/R600/AMDGPUISelLowering.h
> @@ -107,7 +107,7 @@ public:
>
> SDValue LowerIntrinsicIABS(SDValue Op, SelectionDAG &DAG) const;
> SDValue LowerIntrinsicLRP(SDValue Op, SelectionDAG &DAG) const;
> - SDValue LowerMinMax(SDValue Op, SelectionDAG &DAG) const;
> + SDValue CombineMinMax(SDNode *N, SelectionDAG &DAG) const;
> const char* getTargetNodeName(unsigned Opcode) const override;
>
> virtual SDNode *PostISelFolding(MachineSDNode *N,
> diff --git a/lib/Target/R600/R600ISelLowering.cpp b/lib/Target/R600/R600ISelLowering.cpp
> index b40cb67..e3bcab0 100644
> --- a/lib/Target/R600/R600ISelLowering.cpp
> +++ b/lib/Target/R600/R600ISelLowering.cpp
> @@ -986,13 +986,6 @@ SDValue R600TargetLowering::LowerSELECT_CC(SDValue Op, SelectionDAG &DAG) const
> return DAG.getNode(ISD::BITCAST, DL, VT, SelectNode);
> }
>
> -
> - // Possible Min/Max pattern
> - SDValue MinMax = LowerMinMax(Op, DAG);
> - if (MinMax.getNode()) {
> - return MinMax;
> - }
> -
> // If we make it this for it means we have no native instructions to handle
> // this SELECT_CC, so we must lower it.
> SDValue HWTrue, HWFalse;
> @@ -1672,6 +1665,11 @@ SDValue R600TargetLowering::PerformDAGCombine(SDNode *N,
> }
>
> case ISD::SELECT_CC: {
> + // Try common optimizations
> + SDValue Ret = AMDGPUTargetLowering::PerformDAGCombine(N, DCI);
> + if (Ret.getNode())
> + return Ret;
> +
> // fold selectcc (selectcc x, y, a, b, cc), b, a, b, seteq ->
> // selectcc x, y, a, b, inv(cc)
> //
> diff --git a/lib/Target/R600/SIISelLowering.cpp b/lib/Target/R600/SIISelLowering.cpp
> index e6880485..cacff83 100644
> --- a/lib/Target/R600/SIISelLowering.cpp
> +++ b/lib/Target/R600/SIISelLowering.cpp
> @@ -902,12 +902,6 @@ SDValue SITargetLowering::LowerSELECT_CC(SDValue Op, SelectionDAG &DAG) const {
> EVT VT = Op.getValueType();
> SDLoc DL(Op);
>
> - // Possible Min/Max pattern
> - SDValue MinMax = LowerMinMax(Op, DAG);
> - if (MinMax.getNode()) {
> - return MinMax;
> - }
> -
> SDValue Cond = DAG.getNode(ISD::SETCC, DL, MVT::i1, LHS, RHS, CC);
> return DAG.getNode(ISD::SELECT, DL, VT, Cond, True, False);
> }
> diff --git a/test/CodeGen/R600/pv.ll b/test/CodeGen/R600/pv.ll
> index 5a930b2..f322bc7 100644
> --- a/test/CodeGen/R600/pv.ll
> +++ b/test/CodeGen/R600/pv.ll
> @@ -1,7 +1,7 @@
> ; RUN: llc < %s -march=r600 | FileCheck %s
>
> ;CHECK: DOT4 * T{{[0-9]\.W}} (MASKED)
> -;CHECK: MAX T{{[0-9].[XYZW]}}, 0.0, PV.X
> +;CHECK: MAX T{{[0-9].[XYZW]}}, PV.X, 0.0
>
> define void @main(<4 x float> inreg %reg0, <4 x float> inreg %reg1, <4 x float> inreg %reg2, <4 x float> inreg %reg3, <4 x float> inreg %reg4, <4 x float> inreg %reg5, <4 x float> inreg %reg6, <4 x float> inreg %reg7) #0 {
> main_body:
> --
> 1.8.1.5
>
> From 5beacdf941c9c71621a4ae5459d4ab5a1c918bd5 Mon Sep 17 00:00:00 2001
> From: Tom Stellard <thomas.stellard at amd.com>
> Date: Fri, 2 May 2014 23:05:24 -0400
> Subject: [PATCH 2/4] SelectionDAG: Expand SELECT_CC to SELECT + SETCC
>
> 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) &&
> + "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
>
> From bce3eea0bc0d06298601a84658903054da348643 Mon Sep 17 00:00:00 2001
> From: Tom Stellard <thomas.stellard at amd.com>
> Date: Sat, 3 May 2014 01:31:55 -0400
> Subject: [PATCH 3/4] SelectionDAG: Add a helper function to replace
> TLI.isOperationLegalOrCustom()
>
> TLI.isOperationLegalOrCustom() is used everywhere in the DAGCombiner,
> but I think most uses are incorrect. We should not be allowing
> Custom lowered operations to be created after legalization, because
> the targets won't have a chance to custom lower them.
>
> This fixes a bug that is uncovered by the next commit.
> ---
> lib/CodeGen/SelectionDAG/DAGCombiner.cpp | 18 ++++++++++++++++--
> 1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
> index 290f2a1..3e2d7f3 100644
> --- a/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
> +++ b/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
> @@ -178,6 +178,12 @@ namespace {
> SDValue PromoteExtend(SDValue Op);
> bool PromoteLoad(SDValue Op);
>
> + /// \return true if this node will be either expanded or selected by the
> + /// target. This function is preferred to using
> + /// TLI.isOperationLegalOrCustom(), because it takes into account
> + /// whether or not legalization has already happened.
> + bool CanCreateNode(unsigned Opcode, EVT VT) const;
> +
> void ExtendSetCCUses(const SmallVectorImpl<SDNode *> &SetCCs,
> SDValue Trunc, SDValue ExtLoad, SDLoc DL,
> ISD::NodeType ExtType);
> @@ -1063,6 +1069,14 @@ bool DAGCombiner::PromoteLoad(SDValue Op) {
> return false;
> }
>
> +bool DAGCombiner::CanCreateNode(unsigned Opcode, EVT VT) const {
> +
> + // We can create nodes that will be custom lowered before legalization,
> + // but after legalization we can only create legal nodes.
> + return (!LegalOperations && TLI.isOperationLegalOrCustom(Opcode, VT)) ||
> + TLI.isOperationLegal(Opcode, VT);
> +}
> +
>
> //===----------------------------------------------------------------------===//
> // Main DAG Combiner implementation
> @@ -4554,8 +4568,8 @@ SDValue DAGCombiner::visitSELECT(SDNode *N) {
> // Check against MVT::Other for SELECT_CC, which is a workaround for targets
> // having to say they don't support SELECT_CC on every type the DAG knows
> // about, since there is no way to mark an opcode illegal at all value types
> - if (TLI.isOperationLegalOrCustom(ISD::SELECT_CC, MVT::Other) &&
> - TLI.isOperationLegalOrCustom(ISD::SELECT_CC, VT))
> + if (CanCreateNode(ISD::SELECT_CC, MVT::Other) &&
> + CanCreateNode(ISD::SELECT_CC, VT))
> return DAG.getNode(ISD::SELECT_CC, SDLoc(N), VT,
> N0.getOperand(0), N0.getOperand(1),
> N1, N2, N0.getOperand(2));
> --
> 1.8.1.5
>
> From 9b7b204cd72b0f5be34ed20b84c9f1f88b8226fc Mon Sep 17 00:00:00 2001
> From: Tom Stellard <thomas.stellard at amd.com>
> Date: Fri, 2 May 2014 22:31:06 -0400
> Subject: [PATCH 4/4] SelectionDAG: Don't use MVT::Other to determine legality
> of ISD::SELECT_CC
>
> The SelectionDAG bad a special case for ISD::SELECT_CC, where it would
> allow targets to specify:
>
> setOperationAction(ISD::SELECT_CC, MVT::Other, Expand);
>
> to indicate that they wanted to expand ISD::SELECT_CC for all types.
> This wasn't applied correctly everywhere, and it makes writing new
> DAG patterns with ISD::SELECT_CC difficult.
> ---
> lib/CodeGen/SelectionDAG/DAGCombiner.cpp | 19 +++----------------
> lib/Target/ARM/ARMISelLowering.cpp | 4 ++++
> lib/Target/Hexagon/HexagonISelLowering.cpp | 6 ------
> lib/Target/Mips/MipsISelLowering.cpp | 3 ++-
> lib/Target/NVPTX/NVPTXISelLowering.cpp | 8 +++++++-
> lib/Target/X86/X86ISelLowering.cpp | 9 ++++++++-
> lib/Target/XCore/XCoreISelLowering.cpp | 3 ---
> 7 files changed, 24 insertions(+), 28 deletions(-)
>
> diff --git a/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
> index 3e2d7f3..2d3b42a 100644
> --- a/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
> +++ b/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
> @@ -4564,12 +4564,7 @@ SDValue DAGCombiner::visitSELECT(SDNode *N) {
>
> // fold selects based on a setcc into other things, such as min/max/abs
> if (N0.getOpcode() == ISD::SETCC) {
> - // FIXME:
> - // Check against MVT::Other for SELECT_CC, which is a workaround for targets
> - // having to say they don't support SELECT_CC on every type the DAG knows
> - // about, since there is no way to mark an opcode illegal at all value types
> - if (CanCreateNode(ISD::SELECT_CC, MVT::Other) &&
> - CanCreateNode(ISD::SELECT_CC, VT))
> + if (CanCreateNode(ISD::SELECT_CC, VT))
> return DAG.getNode(ISD::SELECT_CC, SDLoc(N), VT,
> N0.getOperand(0), N0.getOperand(1),
> N1, N2, N0.getOperand(2));
> @@ -6964,11 +6959,7 @@ SDValue DAGCombiner::visitSINT_TO_FP(SDNode *N) {
> }
>
> // The next optimizations are desirable only if SELECT_CC can be lowered.
> - // Check against MVT::Other for SELECT_CC, which is a workaround for targets
> - // having to say they don't support SELECT_CC on every type the DAG knows
> - // about, since there is no way to mark an opcode illegal at all value types
> - // (See also visitSELECT)
> - if (TLI.isOperationLegalOrCustom(ISD::SELECT_CC, MVT::Other)) {
> + if (TLI.isOperationLegalOrCustom(ISD::SELECT_CC, VT) || !LegalOperations) {
> // fold (sint_to_fp (setcc x, y, cc)) -> (select_cc x, y, -1.0, 0.0,, cc)
> if (N0.getOpcode() == ISD::SETCC && N0.getValueType() == MVT::i1 &&
> !VT.isVector() &&
> @@ -7021,11 +7012,7 @@ SDValue DAGCombiner::visitUINT_TO_FP(SDNode *N) {
> }
>
> // The next optimizations are desirable only if SELECT_CC can be lowered.
> - // Check against MVT::Other for SELECT_CC, which is a workaround for targets
> - // having to say they don't support SELECT_CC on every type the DAG knows
> - // about, since there is no way to mark an opcode illegal at all value types
> - // (See also visitSELECT)
> - if (TLI.isOperationLegalOrCustom(ISD::SELECT_CC, MVT::Other)) {
> + if (TLI.isOperationLegalOrCustom(ISD::SELECT_CC, VT) || !LegalOperations) {
> // fold (uint_to_fp (setcc x, y, cc)) -> (select_cc x, y, -1.0, 0.0,, cc)
>
> if (N0.getOpcode() == ISD::SETCC && !VT.isVector() &&
> diff --git a/lib/Target/ARM/ARMISelLowering.cpp b/lib/Target/ARM/ARMISelLowering.cpp
> index e1e403c..832869f 100644
> --- a/lib/Target/ARM/ARMISelLowering.cpp
> +++ b/lib/Target/ARM/ARMISelLowering.cpp
> @@ -47,6 +47,9 @@
> #include "llvm/Support/MathExtras.h"
> #include "llvm/Target/TargetOptions.h"
> #include <utility>
> +
> +#include "llvm/Support/Debug.h"
> +
> using namespace llvm;
>
> #define DEBUG_TYPE "arm-isel"
> @@ -116,6 +119,7 @@ void ARMTargetLowering::addTypeForNEON(MVT VT, MVT PromotedLdStVT,
> setOperationAction(ISD::CONCAT_VECTORS, VT, Legal);
> setOperationAction(ISD::EXTRACT_SUBVECTOR, VT, Legal);
> setOperationAction(ISD::SELECT, VT, Expand);
> + dbgs() << "Expanding SELECT_CC for " << EVT(VT).getEVTString() << "\n";
> setOperationAction(ISD::SELECT_CC, VT, Expand);
> setOperationAction(ISD::VSELECT, VT, Expand);
> setOperationAction(ISD::SIGN_EXTEND_INREG, VT, Expand);
> diff --git a/lib/Target/Hexagon/HexagonISelLowering.cpp b/lib/Target/Hexagon/HexagonISelLowering.cpp
> index aa4e9b5..394a552 100644
> --- a/lib/Target/Hexagon/HexagonISelLowering.cpp
> +++ b/lib/Target/Hexagon/HexagonISelLowering.cpp
> @@ -1339,18 +1339,12 @@ HexagonTargetLowering::HexagonTargetLowering(HexagonTargetMachine
>
> setOperationAction(ISD::SELECT_CC, MVT::f32, Expand);
> setOperationAction(ISD::SELECT_CC, MVT::f64, Expand);
> - setOperationAction(ISD::SELECT_CC, MVT::Other, Expand);
>
> } else {
>
> // Hexagon has no select or setcc: expand to SELECT_CC.
> setOperationAction(ISD::SELECT, MVT::f32, Expand);
> setOperationAction(ISD::SELECT, MVT::f64, Expand);
> -
> - // This is a workaround documented in DAGCombiner.cpp:2892 We don't
> - // support SELECT_CC on every type.
> - setOperationAction(ISD::SELECT_CC, MVT::Other, Expand);
> -
> }
>
> if (EmitJumpTables) {
> diff --git a/lib/Target/Mips/MipsISelLowering.cpp b/lib/Target/Mips/MipsISelLowering.cpp
> index d152e67..9dcc772 100644
> --- a/lib/Target/Mips/MipsISelLowering.cpp
> +++ b/lib/Target/Mips/MipsISelLowering.cpp
> @@ -287,7 +287,8 @@ MipsTargetLowering::MipsTargetLowering(MipsTargetMachine &TM)
> setOperationAction(ISD::BR_CC, MVT::f64, Expand);
> setOperationAction(ISD::BR_CC, MVT::i32, Expand);
> setOperationAction(ISD::BR_CC, MVT::i64, Expand);
> - setOperationAction(ISD::SELECT_CC, MVT::Other, Expand);
> + setOperationAction(ISD::SELECT_CC, MVT::i32, Expand);
> + setOperationAction(ISD::SELECT_CC, MVT::i64, Expand);
> setOperationAction(ISD::UINT_TO_FP, MVT::i32, Expand);
> setOperationAction(ISD::UINT_TO_FP, MVT::i64, Expand);
> setOperationAction(ISD::FP_TO_UINT, MVT::i32, Expand);
> diff --git a/lib/Target/NVPTX/NVPTXISelLowering.cpp b/lib/Target/NVPTX/NVPTXISelLowering.cpp
> index e6860a9..91e6dac 100644
> --- a/lib/Target/NVPTX/NVPTXISelLowering.cpp
> +++ b/lib/Target/NVPTX/NVPTXISelLowering.cpp
> @@ -130,7 +130,13 @@ NVPTXTargetLowering::NVPTXTargetLowering(NVPTXTargetMachine &TM)
> addRegisterClass(MVT::f64, &NVPTX::Float64RegsRegClass);
>
> // Operations not directly supported by NVPTX.
> - setOperationAction(ISD::SELECT_CC, MVT::Other, Expand);
> + setOperationAction(ISD::SELECT_CC, MVT::f32, Expand);
> + setOperationAction(ISD::SELECT_CC, MVT::f64, Expand);
> + setOperationAction(ISD::SELECT_CC, MVT::i1, Expand);
> + setOperationAction(ISD::SELECT_CC, MVT::i8, Expand);
> + setOperationAction(ISD::SELECT_CC, MVT::i16, Expand);
> + setOperationAction(ISD::SELECT_CC, MVT::i32, Expand);
> + setOperationAction(ISD::SELECT_CC, MVT::i64, Expand);
> setOperationAction(ISD::BR_CC, MVT::f32, Expand);
> setOperationAction(ISD::BR_CC, MVT::f64, Expand);
> setOperationAction(ISD::BR_CC, MVT::i1, Expand);
> diff --git a/lib/Target/X86/X86ISelLowering.cpp b/lib/Target/X86/X86ISelLowering.cpp
> index 0ed30f5..0362bce 100644
> --- a/lib/Target/X86/X86ISelLowering.cpp
> +++ b/lib/Target/X86/X86ISelLowering.cpp
> @@ -442,7 +442,13 @@ void X86TargetLowering::resetOperationActions() {
> setOperationAction(ISD::BR_CC , MVT::i16, Expand);
> setOperationAction(ISD::BR_CC , MVT::i32, Expand);
> setOperationAction(ISD::BR_CC , MVT::i64, Expand);
> - setOperationAction(ISD::SELECT_CC , MVT::Other, Expand);
> + setOperationAction(ISD::SELECT_CC , MVT::f32, Expand);
> + setOperationAction(ISD::SELECT_CC , MVT::f64, Expand);
> + setOperationAction(ISD::SELECT_CC , MVT::f80, Expand);
> + setOperationAction(ISD::SELECT_CC , MVT::i8, Expand);
> + setOperationAction(ISD::SELECT_CC , MVT::i16, Expand);
> + setOperationAction(ISD::SELECT_CC , MVT::i32, Expand);
> + setOperationAction(ISD::SELECT_CC , MVT::i64, Expand);
> if (Subtarget->is64Bit())
> setOperationAction(ISD::SIGN_EXTEND_INREG, MVT::i32, Legal);
> setOperationAction(ISD::SIGN_EXTEND_INREG, MVT::i16 , Legal);
> @@ -860,6 +866,7 @@ void X86TargetLowering::resetOperationActions() {
> setOperationAction(ISD::ZERO_EXTEND, VT, Expand);
> setOperationAction(ISD::ANY_EXTEND, VT, Expand);
> setOperationAction(ISD::VSELECT, VT, Expand);
> + setOperationAction(ISD::SELECT_CC, VT, Expand);
> for (int InnerVT = MVT::FIRST_VECTOR_VALUETYPE;
> InnerVT <= MVT::LAST_VECTOR_VALUETYPE; ++InnerVT)
> setTruncStoreAction(VT,
> diff --git a/lib/Target/XCore/XCoreISelLowering.cpp b/lib/Target/XCore/XCoreISelLowering.cpp
> index 96eebb4..37f53c5 100644
> --- a/lib/Target/XCore/XCoreISelLowering.cpp
> +++ b/lib/Target/XCore/XCoreISelLowering.cpp
> @@ -98,9 +98,6 @@ XCoreTargetLowering::XCoreTargetLowering(XCoreTargetMachine &XTM)
> setOperationAction(ISD::SUBC, MVT::i32, Expand);
> setOperationAction(ISD::SUBE, MVT::i32, Expand);
>
> - // Stop the combiner recombining select and set_cc
> - setOperationAction(ISD::SELECT_CC, MVT::Other, Expand);
> -
> // 64bit
> setOperationAction(ISD::ADD, MVT::i64, Custom);
> setOperationAction(ISD::SUB, MVT::i64, Custom);
> --
> 1.8.1.5
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-SelectionDAG-Enable-and-setcc-x-setcc-y-setcc-and-x-.patch
Type: text/x-diff
Size: 2318 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140505/ca2803d0/attachment.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-SelectionDAG-Don-t-use-MVT-Other-to-determine-legali.patch
Type: text/x-diff
Size: 9293 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140505/ca2803d0/attachment-0001.patch>
More information about the llvm-commits
mailing list