PATCH: DAGCombiner - Use correct value type when checking legality of BR_CC - Changes to NVPTX, Mips, Hexagon, MBlaze, X86, and XCore targets
Tom Stellard
tom at stellard.net
Wed Mar 6 14:11:40 PST 2013
Hi Nadav,
How do the X86 changes in this patch look to you? Is it OK to commit?
Thanks,
Tom
On Mon, Mar 04, 2013 at 11:36:00PM +0100, Tom Stellard wrote:
> Hi,
>
> Here is an updated patches with the requested fixes for Hexagon, Mips,
> and XCore targets.
>
> -Tom
>
> On Mon, Mar 04, 2013 at 09:03:25AM +0000, Richard Osborne wrote:
> > Hi Tom,
> >
> > For XCore the you only need to expand i32 - f32 isn't a legal type so setting expand on it isn't necessary.
> >
> > On 27 Feb 2013, at 23:14, Tom Stellard <tom at stellard.net> wrote:
> >
> > > Here is an updated patch. The previous version had some unrelated
> > > changes included in it.
> > >
> > > -Tom
> > >
> > > On Wed, Feb 27, 2013 at 05:21:59PM -0500, Tom Stellard wrote:
> > >> Forgot the patch...
> > >>
> > >> On Wed, Feb 27, 2013 at 05:17:32PM -0500, Tom Stellard wrote:
> > >>> Hi,
> > >>>
> > >>> The attached patch fixes a bug where the DAGCombiner was passing the
> > >>> MVT::Other value type to TLI.isOperationLegalOrCustom() for a BR_CC node.
> > >>> LegalizeDAG uses the value type of the comparison operands, and I think
> > >>> this is what the DAGCombiner should be using as well.
> > >>>
> > >>> I've updated the *ISelLowering files for the affected targets, but
> > >>> backend code owners should double check my work.
> > >>>
> > >>> Thanks,
> > >>> Tom
> > >>> _______________________________________________
> > >>> llvm-commits mailing list
> > >>> llvm-commits at cs.uiuc.edu
> > >>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> > >
> > > <0001-DAGCombiner-Use-correct-value-type-for-checking-lega.patch>_______________________________________________
> > > llvm-commits mailing list
> > > llvm-commits at cs.uiuc.edu
> > > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >
> From 79413d8cf55805a5d6b5f46118677dd9e1a224a9 Mon Sep 17 00:00:00 2001
> From: Tom Stellard <thomas.stellard at amd.com>
> Date: Wed, 27 Feb 2013 17:03:48 -0500
> Subject: [PATCH] DAGCombiner: Use correct value type for checking legality of
> BR_CC v3
>
> LegalizeDAG.cpp uses the value of the comparison operands when checking
> the legality of BR_CC, so DAGCombiner should do the same.
>
> v2:
> - Expand more BR_CC value types for NVPTX
>
> v3:
> - Expand correct BR_CC value types for Hexagon, Mips, and XCore.
> ---
> lib/CodeGen/SelectionDAG/DAGCombiner.cpp | 3 ++-
> lib/Target/Hexagon/HexagonISelLowering.cpp | 4 +++-
> lib/Target/MBlaze/MBlazeISelLowering.cpp | 3 ++-
> lib/Target/Mips/MipsISelLowering.cpp | 5 ++++-
> lib/Target/NVPTX/NVPTXISelLowering.cpp | 8 +++++++-
> lib/Target/X86/X86ISelLowering.cpp | 8 +++++++-
> lib/Target/XCore/XCoreISelLowering.cpp | 2 +-
> 7 files changed, 26 insertions(+), 7 deletions(-)
>
> diff --git a/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
> index ec52d7e..716fb93 100644
> --- a/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
> +++ b/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
> @@ -6709,7 +6709,8 @@ SDValue DAGCombiner::visitBRCOND(SDNode *N) {
> // fold a brcond with a setcc condition into a BR_CC node if BR_CC is legal
> // on the target.
> if (N1.getOpcode() == ISD::SETCC &&
> - TLI.isOperationLegalOrCustom(ISD::BR_CC, MVT::Other)) {
> + TLI.isOperationLegalOrCustom(ISD::BR_CC,
> + N1.getOperand(0).getValueType())) {
> return DAG.getNode(ISD::BR_CC, N->getDebugLoc(), MVT::Other,
> Chain, N1.getOperand(2),
> N1.getOperand(0), N1.getOperand(1), N2);
> diff --git a/lib/Target/Hexagon/HexagonISelLowering.cpp b/lib/Target/Hexagon/HexagonISelLowering.cpp
> index fac931a..6515de8 100644
> --- a/lib/Target/Hexagon/HexagonISelLowering.cpp
> +++ b/lib/Target/Hexagon/HexagonISelLowering.cpp
> @@ -1342,7 +1342,6 @@ HexagonTargetLowering::HexagonTargetLowering(HexagonTargetMachine
>
> }
>
> - setOperationAction(ISD::BR_CC, MVT::Other, Expand);
> setOperationAction(ISD::BRIND, MVT::Other, Expand);
> if (EmitJumpTables) {
> setOperationAction(ISD::BR_JT, MVT::Other, Custom);
> @@ -1352,6 +1351,9 @@ HexagonTargetLowering::HexagonTargetLowering(HexagonTargetMachine
> // Increase jump tables cutover to 5, was 4.
> setMinimumJumpTableEntries(5);
>
> + setOperationAction(ISD::BR_CC, MVT::f32, Expand);
> + setOperationAction(ISD::BR_CC, MVT::f64, Expand);
> + setOperationAction(ISD::BR_CC, MVT::i1, Expand);
> setOperationAction(ISD::BR_CC, MVT::i32, Expand);
>
> setOperationAction(ISD::MEMBARRIER, MVT::Other, Custom);
> diff --git a/lib/Target/MBlaze/MBlazeISelLowering.cpp b/lib/Target/MBlaze/MBlazeISelLowering.cpp
> index 7664c60..d4f9432 100644
> --- a/lib/Target/MBlaze/MBlazeISelLowering.cpp
> +++ b/lib/Target/MBlaze/MBlazeISelLowering.cpp
> @@ -160,7 +160,8 @@ MBlazeTargetLowering::MBlazeTargetLowering(MBlazeTargetMachine &TM)
> // Operations not directly supported by MBlaze.
> setOperationAction(ISD::DYNAMIC_STACKALLOC, MVT::i32, Expand);
> setOperationAction(ISD::BR_JT, MVT::Other, Expand);
> - setOperationAction(ISD::BR_CC, MVT::Other, Expand);
> + setOperationAction(ISD::BR_CC, MVT::f32, Expand);
> + setOperationAction(ISD::BR_CC, MVT::i32, Expand);
> setOperationAction(ISD::SIGN_EXTEND_INREG, MVT::i1, Expand);
> setOperationAction(ISD::ROTL, MVT::i32, Expand);
> setOperationAction(ISD::ROTR, MVT::i32, Expand);
> diff --git a/lib/Target/Mips/MipsISelLowering.cpp b/lib/Target/Mips/MipsISelLowering.cpp
> index 310a82e..896d3d7 100644
> --- a/lib/Target/Mips/MipsISelLowering.cpp
> +++ b/lib/Target/Mips/MipsISelLowering.cpp
> @@ -398,7 +398,10 @@ MipsTargetLowering(MipsTargetMachine &TM)
>
> // Operations not directly supported by Mips.
> setOperationAction(ISD::BR_JT, MVT::Other, Expand);
> - setOperationAction(ISD::BR_CC, MVT::Other, Expand);
> + setOperationAction(ISD::BR_CC, MVT::f32, Expand);
> + 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::UINT_TO_FP, MVT::i32, Expand);
> setOperationAction(ISD::UINT_TO_FP, MVT::i64, Expand);
> diff --git a/lib/Target/NVPTX/NVPTXISelLowering.cpp b/lib/Target/NVPTX/NVPTXISelLowering.cpp
> index 5ee747a..b64562c 100644
> --- a/lib/Target/NVPTX/NVPTXISelLowering.cpp
> +++ b/lib/Target/NVPTX/NVPTXISelLowering.cpp
> @@ -101,7 +101,13 @@ NVPTXTargetLowering::NVPTXTargetLowering(NVPTXTargetMachine &TM)
>
> // Operations not directly supported by NVPTX.
> setOperationAction(ISD::SELECT_CC, MVT::Other, Expand);
> - setOperationAction(ISD::BR_CC, MVT::Other, Expand);
> + setOperationAction(ISD::BR_CC, MVT::f32, Expand);
> + setOperationAction(ISD::BR_CC, MVT::f64, Expand);
> + setOperationAction(ISD::BR_CC, MVT::i1, Expand);
> + setOperationAction(ISD::BR_CC, MVT::i8, Expand);
> + setOperationAction(ISD::BR_CC, MVT::i16, Expand);
> + setOperationAction(ISD::BR_CC, MVT::i32, Expand);
> + setOperationAction(ISD::BR_CC, MVT::i64, Expand);
> setOperationAction(ISD::SIGN_EXTEND_INREG, MVT::i64, Expand);
> setOperationAction(ISD::SIGN_EXTEND_INREG, MVT::i32, Expand);
> setOperationAction(ISD::SIGN_EXTEND_INREG, MVT::i16, Expand);
> diff --git a/lib/Target/X86/X86ISelLowering.cpp b/lib/Target/X86/X86ISelLowering.cpp
> index 451acca..15f3777 100644
> --- a/lib/Target/X86/X86ISelLowering.cpp
> +++ b/lib/Target/X86/X86ISelLowering.cpp
> @@ -368,7 +368,13 @@ X86TargetLowering::X86TargetLowering(X86TargetMachine &TM)
>
> setOperationAction(ISD::BR_JT , MVT::Other, Expand);
> setOperationAction(ISD::BRCOND , MVT::Other, Custom);
> - setOperationAction(ISD::BR_CC , MVT::Other, Expand);
> + setOperationAction(ISD::BR_CC , MVT::f32, Expand);
> + setOperationAction(ISD::BR_CC , MVT::f64, Expand);
> + setOperationAction(ISD::BR_CC , MVT::f80, Expand);
> + setOperationAction(ISD::BR_CC , MVT::i8, Expand);
> + 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);
> if (Subtarget->is64Bit())
> setOperationAction(ISD::SIGN_EXTEND_INREG, MVT::i32, Legal);
> diff --git a/lib/Target/XCore/XCoreISelLowering.cpp b/lib/Target/XCore/XCoreISelLowering.cpp
> index f8a9125..a5d2be8 100644
> --- a/lib/Target/XCore/XCoreISelLowering.cpp
> +++ b/lib/Target/XCore/XCoreISelLowering.cpp
> @@ -84,7 +84,7 @@ XCoreTargetLowering::XCoreTargetLowering(XCoreTargetMachine &XTM)
> setBooleanVectorContents(ZeroOrOneBooleanContent); // FIXME: Is this correct?
>
> // XCore does not have the NodeTypes below.
> - setOperationAction(ISD::BR_CC, MVT::Other, Expand);
> + setOperationAction(ISD::BR_CC, MVT::i32, Expand);
> setOperationAction(ISD::SELECT_CC, MVT::i32, Custom);
> setOperationAction(ISD::ADDC, MVT::i32, Expand);
> setOperationAction(ISD::ADDE, MVT::i32, Expand);
> --
> 1.7.11.4
>
> _______________________________________________
> 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