On Wed, Dec 05, 2012 at 04:58:21PM +0100, Vincent Lejeune wrote:
> ---
> lib/Target/AMDGPU/AMDGPUISelLowering.cpp | 53
> ++++++++++++++++++++++++++++++++
> lib/Target/AMDGPU/AMDGPUISelLowering.h | 1 +
> lib/Target/AMDGPU/R600ISelLowering.cpp | 5 +++
> lib/Target/AMDGPU/SIISelLowering.cpp | 6 ++++
> 4 files changed, 65 insertions(+)
>
Hi Vincent,
This looks much better, thanks. Just a few minor issues, but with those fixed,
this patch is:
Reviewed-by: Tom Stellard <thomas.stell...@amd.com>
> diff --git a/lib/Target/AMDGPU/AMDGPUISelLowering.cpp
> b/lib/Target/AMDGPU/AMDGPUISelLowering.cpp
> index 5bde9db..4c1a070 100644
> --- a/lib/Target/AMDGPU/AMDGPUISelLowering.cpp
> +++ b/lib/Target/AMDGPU/AMDGPUISelLowering.cpp
> @@ -172,7 +172,60 @@ SDValue AMDGPUTargetLowering::LowerIntrinsicLRP(SDValue
> Op,
> OneSubAC);
> }
>
> +/// Generate Min/Max pattern
> +SDValue AMDGPUTargetLowering::LowerMinMax(SDValue Op,
> + SelectionDAG &DAG) const
> +{
Brace needs to be on the same line as the function here
> + DebugLoc DL = Op.getDebugLoc();
> + EVT VT = Op.getValueType();
>
> + SDValue LHS = Op.getOperand(0);
> + SDValue RHS = Op.getOperand(1);
> + SDValue True = Op.getOperand(2);
> + SDValue CC = Op.getOperand(4);
You should move this check into the function:
EVT CompareVT = LHS.getValueType();
if (CompareVT != VT || VT != MVT::f32 ||
(LHS != True && LHS != False) ||
(RHS != True && RHS != False)) {
return SDValue();
}
Make sure to double check my logic here.
> + ISD::CondCode CCOpcode = cast<CondCodeSDNode>(CC)->get();
> + switch (CCOpcode) {
> + case ISD::SETOEQ:
> + case ISD::SETONE:
> + case ISD::SETUNE:
> + case ISD::SETNE:
> + case ISD::SETUEQ:
> + case ISD::SETEQ:
> + case ISD::SETFALSE:
> + case ISD::SETFALSE2:
> + case ISD::SETTRUE:
> + case ISD::SETTRUE2:
> + case ISD::SETUO:
> + case ISD::SETO:
> + assert(0 && "Operation should already be optimised !");
> + case ISD::SETULE:
> + case ISD::SETULT:
> + case ISD::SETOLE:
> + case ISD::SETOLT:
> + case ISD::SETLE:
> + case ISD::SETLT: {
> + if (LHS == True)
> + return DAG.getNode(AMDGPUISD::FMIN, DL, VT, LHS, RHS);
> + else
> + return DAG.getNode(AMDGPUISD::FMAX, DL, VT, LHS, RHS);
> + }
> + case ISD::SETGT:
> + case ISD::SETGE:
> + case ISD::SETUGE:
> + case ISD::SETOGE:
> + case ISD::SETUGT:
> + case ISD::SETOGT: {
> + if (LHS == True)
> + return DAG.getNode(AMDGPUISD::FMAX, DL, VT, LHS, RHS);
> + else
> + return DAG.getNode(AMDGPUISD::FMIN, DL, VT, LHS, RHS);
> + }
> + case ISD::SETCC_INVALID:
> + assert(0 && "Invalid setcc condcode !");
> + }
> + return Op;
> +}
>
> SDValue AMDGPUTargetLowering::LowerUDIVREM(SDValue Op,
> SelectionDAG &DAG) const
> diff --git a/lib/Target/AMDGPU/AMDGPUISelLowering.h
> b/lib/Target/AMDGPU/AMDGPUISelLowering.h
> index 60de190..3b60ae1 100644
> --- a/lib/Target/AMDGPU/AMDGPUISelLowering.h
> +++ b/lib/Target/AMDGPU/AMDGPUISelLowering.h
> @@ -56,6 +56,7 @@ public:
> virtual SDValue LowerOperation(SDValue Op, SelectionDAG &DAG) const;
> SDValue LowerIntrinsicIABS(SDValue Op, SelectionDAG &DAG) const;
> SDValue LowerIntrinsicLRP(SDValue Op, SelectionDAG &DAG) const;
> + SDValue LowerMinMax(SDValue Op, SelectionDAG &DAG) const;
> virtual const char* getTargetNodeName(unsigned Opcode) const;
>
> // Functions defined in AMDILISelLowering.cpp
> diff --git a/lib/Target/AMDGPU/R600ISelLowering.cpp
> b/lib/Target/AMDGPU/R600ISelLowering.cpp
> index 6f1c1d7..374fbfc 100644
> --- a/lib/Target/AMDGPU/R600ISelLowering.cpp
> +++ b/lib/Target/AMDGPU/R600ISelLowering.cpp
> @@ -764,6 +764,11 @@ SDValue R600TargetLowering::LowerSELECT_CC(SDValue Op,
> SelectionDAG &DAG) const
> }
> }
>
> + if (CompareVT == VT == MVT::f32 &&
> + ((LHS == True && RHS == False) || (LHS == False && RHS == True))) {
> + return LowerMinMax(Op, DAG);
> + }
> +
With this check in the common function, replace the above code with:
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;
> diff --git a/lib/Target/AMDGPU/SIISelLowering.cpp
> b/lib/Target/AMDGPU/SIISelLowering.cpp
> index 45f180f..98a7376 100644
> --- a/lib/Target/AMDGPU/SIISelLowering.cpp
> +++ b/lib/Target/AMDGPU/SIISelLowering.cpp
> @@ -383,6 +383,12 @@ SDValue SITargetLowering::LowerSELECT_CC(SDValue Op,
> SelectionDAG &DAG) const
> EVT VT = Op.getValueType();
> DebugLoc DL = Op.getDebugLoc();
>
> + EVT CompareVT = LHS.getValueType();
> + if (CompareVT == VT == MVT::f32 &&
> + ((LHS == True && RHS == False) || (LHS == False && RHS == True))) {
> + return LowerMinMax(Op, DAG);
> + }
> +
You can do the same replacement here too.
> SDValue Cond = DAG.getNode(ISD::SETCC, DL, MVT::i1, LHS, RHS, CC);
> return DAG.getNode(ISD::SELECT, DL, VT, Cond, True, False);
> }
> --
> 1.8.0.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
