[PATCH 1/4] R600: Simplify LowerUDIVREM

[Tom Stellard]

On Thu, Oct 16, 2014 at 02:41:42PM -0400, Jan Vesely wrote:
> optimizations can handle removing the Hi part operations.
> The generated code is identical

Hi Jan,

This is somewhat unrelated to this patch, but I think there are some
bugs in the lowering for 64-bit division.

See patch #5:
R600: Factor i64 UDIVREM lowering into its own fuction

>From this email:
http://marc.info/?l=llvm-commits&m=141150582511354&w=2

One of the failing cases is:

0x1 / 0xffffffffffffffff = 0xffffffff (should be 0x0).

The other patches in that series add a pass to use the
LLVM IR based division lowering, which works fine on
at least on SI.

It would be great if we could fix the SDAG lowering version
since that would potentially produce much better code, otherwise
I can continue with the LLVM IR base lowering approach.

If you do try to fix this, I think it would be good to apply the
patch referenced above first to make it easier to share code between
SI and R600.

-Tom

> 
> Signed-off-by: Jan Vesely <jan.vesely at rutgers.edu>
> ---
>  lib/Target/R600/R600ISelLowering.cpp | 30 +++++++++++-------------------
>  1 file changed, 11 insertions(+), 19 deletions(-)
> 
> diff --git a/lib/Target/R600/R600ISelLowering.cpp b/lib/Target/R600/R600ISelLowering.cpp
> index 3b4aed0..90a1f2f 100644
> --- a/lib/Target/R600/R600ISelLowering.cpp
> +++ b/lib/Target/R600/R600ISelLowering.cpp
> @@ -893,8 +893,8 @@ void R600TargetLowering::ReplaceNodeResults(SDNode *N,
>      SDValue DIV_Part = DAG.getNode(ISD::UDIV, DL, HalfVT, LHS_Hi, RHS_Lo);
>      SDValue REM_Part = DAG.getNode(ISD::UREM, DL, HalfVT, LHS_Hi, RHS_Lo);
>  
> -    SDValue REM_Hi = zero;
>      SDValue REM_Lo = DAG.getSelectCC(DL, RHS_Hi, zero, REM_Part, LHS_Hi, ISD::SETEQ);
> +    SDValue REM = DAG.getNode(ISD::BUILD_PAIR, DL, VT, REM_Lo, zero);
>  
>      SDValue DIV_Hi = DAG.getSelectCC(DL, RHS_Hi, zero, DIV_Part, zero, ISD::SETEQ);
>      SDValue DIV_Lo = zero;
> @@ -902,8 +902,10 @@ void R600TargetLowering::ReplaceNodeResults(SDNode *N,
>      const unsigned halfBitWidth = HalfVT.getSizeInBits();
>  
>      for (unsigned i = 0; i < halfBitWidth; ++i) {
> -      SDValue POS = DAG.getConstant(halfBitWidth - i - 1, HalfVT);
> -      // Get Value of high bit
> +      const unsigned bitPos = halfBitWidth - i - 1;
> +      SDValue POS = DAG.getConstant(bitPos, HalfVT);
> +      // Get value of high bit
> +      // TODO: Remove the BFE part when the optimization is fixed
>        SDValue HBit;
>        if (halfBitWidth == 32 && Subtarget->hasBFE()) {
>          HBit = DAG.getNode(AMDGPUISD::BFE_U32, DL, HalfVT, LHS_Lo, POS, one);
> @@ -911,33 +913,23 @@ void R600TargetLowering::ReplaceNodeResults(SDNode *N,
>          HBit = DAG.getNode(ISD::SRL, DL, HalfVT, LHS_Lo, POS);
>          HBit = DAG.getNode(ISD::AND, DL, HalfVT, HBit, one);
>        }
> +      HBit = DAG.getNode(ISD::ZERO_EXTEND, DL, VT, HBit);
>  
> -      SDValue Carry = DAG.getNode(ISD::SRL, DL, HalfVT, REM_Lo,
> -        DAG.getConstant(halfBitWidth - 1, HalfVT));
> -      REM_Hi = DAG.getNode(ISD::SHL, DL, HalfVT, REM_Hi, one);
> -      REM_Hi = DAG.getNode(ISD::OR, DL, HalfVT, REM_Hi, Carry);
> +      // Shift
> +      REM = DAG.getNode(ISD::SHL, DL, VT, REM, DAG.getConstant(1, VT));
> +      // Add LHS high bit
> +      REM = DAG.getNode(ISD::OR, DL, VT, REM, HBit);
>  
> -      REM_Lo = DAG.getNode(ISD::SHL, DL, HalfVT, REM_Lo, one);
> -      REM_Lo = DAG.getNode(ISD::OR, DL, HalfVT, REM_Lo, HBit);
> -
> -
> -      SDValue REM = DAG.getNode(ISD::BUILD_PAIR, DL, VT, REM_Lo, REM_Hi);
> -
> -      SDValue BIT = DAG.getConstant(1 << (halfBitWidth - i - 1), HalfVT);
> +      SDValue BIT = DAG.getConstant(1 << bitPos, HalfVT);
>        SDValue realBIT = DAG.getSelectCC(DL, REM, RHS, BIT, zero, ISD::SETGE);
>  
>        DIV_Lo = DAG.getNode(ISD::OR, DL, HalfVT, DIV_Lo, realBIT);
>  
>        // Update REM
> -
>        SDValue REM_sub = DAG.getNode(ISD::SUB, DL, VT, REM, RHS);
> -
>        REM = DAG.getSelectCC(DL, REM, RHS, REM_sub, REM, ISD::SETGE);
> -      REM_Lo = DAG.getNode(ISD::EXTRACT_ELEMENT, DL, HalfVT, REM, zero);
> -      REM_Hi = DAG.getNode(ISD::EXTRACT_ELEMENT, DL, HalfVT, REM, one);
>      }
>  
> -    SDValue REM = DAG.getNode(ISD::BUILD_PAIR, DL, VT, REM_Lo, REM_Hi);
>      SDValue DIV = DAG.getNode(ISD::BUILD_PAIR, DL, VT, DIV_Lo, DIV_Hi);
>      Results.push_back(DIV);
>      Results.push_back(REM);
> -- 
> 1.9.3
>