[llvm] r336664 - [DAGCombiner] visitREM - call visitSDIVLike/visitUDIVLike directly to avoid recursive combining.

[Friedman, Eli via llvm-commits]

On 7/10/2018 6:18 AM, Simon Pilgrim via llvm-commits wrote:
> Author: rksimon
> Date: Tue Jul 10 06:18:16 2018
> New Revision: 336664
>
> URL: http://llvm.org/viewvc/llvm-project?rev=336664&view=rev
> Log:
> [DAGCombiner] visitREM - call visitSDIVLike/visitUDIVLike directly to avoid recursive combining.
>
> As suggested by @efriedma on D48975 use the visitSDIVLike/visitUDIVLike functions introduced at rL336656.
>
> Modified:
>      llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
>
> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp?rev=336664&r1=336663&r2=336664&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (original)
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp Tue Jul 10 06:18:16 2018
> @@ -3277,22 +3277,19 @@ SDValue DAGCombiner::visitREM(SDNode *N)
>   
>     // If X/C can be simplified by the division-by-constant logic, lower
>     // X%C to the equivalent of X-X/C*C.
> -  // To avoid mangling nodes, this simplification requires that the combine()
> -  // call for the speculative DIV must not cause a DIVREM conversion.  We guard
> -  // against this by skipping the simplification if isIntDivCheap().  When
> -  // div is not cheap, combine will not return a DIVREM.  Regardless,
> -  // checking cheapness here makes sense since the simplification results in
> -  // fatter code.
> +  // Reuse the SDIVLike/UDIVLike combines - to avoid mangling nodes, the
> +  // speculative DIV must not cause a DIVREM conversion.  We guard against this
> +  // by skipping the simplification if isIntDivCheap().  When div is not cheap,
> +  // combine will not return a DIVREM.  Regardless, checking cheapness here
> +  // makes sense since the simplification results in fatter code.
>     if (N1C && !N1C->isNullValue() && !TLI.isIntDivCheap(VT, Attr)) {
> -    unsigned DivOpcode = isSigned ? ISD::SDIV : ISD::UDIV;
> -    SDValue Div = DAG.getNode(DivOpcode, DL, VT, N0, N1);
> -    AddToWorklist(Div.getNode());
> -    SDValue OptimizedDiv = combine(Div.getNode());
> -    if (OptimizedDiv.getNode() && OptimizedDiv.getNode() != Div.getNode() &&
> -        OptimizedDiv.getOpcode() != ISD::UDIVREM &&
> +    SDValue OptimizedDiv =
> +        isSigned ? visitSDIVLike(N0, N1, N) : visitUDIVLike(N0, N1, N);
> +    if (OptimizedDiv.getNode() && OptimizedDiv.getOpcode() != ISD::UDIVREM &&
>           OptimizedDiv.getOpcode() != ISD::SDIVREM) {

I think the check for SDIVREM/UDIVREM is redundant here? visitUDIVLike 
can't return UDIVREM.

-Eli

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project