[llvm] r329350 - [InstCombine] nsz: -(X - Y) --> Y - X

[via llvm-commits]

This patch is giving us fairly significant instruction count regressions on a number of shaders. The regressions go away if we gate it with hasOneUse, with makes sense; the patch as committed is duplicating arithmetic!  Is it possible to add hasOneUse checks to this? I don’t feel like it’s reasonable to duplicate fsubs just because they have a negated use, especially since the compiler as a whole is very bad at deduplicating them later.

—escha

> On Apr 5, 2018, at 2:37 PM, Sanjay Patel via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> Author: spatel
> Date: Thu Apr  5 14:37:17 2018
> New Revision: 329350
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=329350&view=rev
> Log:
> [InstCombine] nsz: -(X - Y) --> Y - X
> 
> This restores part of the fold that was removed with rL73243 (PR4374).
> 
> Modified:
>    llvm/trunk/lib/Transforms/InstCombine/InstCombineAddSub.cpp
>    llvm/trunk/test/Transforms/InstCombine/fsub.ll
> 
> Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineAddSub.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineAddSub.cpp?rev=329350&r1=329349&r2=329350&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/InstCombine/InstCombineAddSub.cpp (original)
> +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineAddSub.cpp Thu Apr  5 14:37:17 2018
> @@ -1698,10 +1698,17 @@ Instruction *InstCombiner::visitFSub(Bin
>                                   SQ.getWithInstruction(&I)))
>     return replaceInstUsesWith(I, V);
> 
> -  // Subtraction from -0.0 is the canonical form of fneg.
> -  // fsub nsz 0, X ==> fsub nsz -0.0, X
> -  if (I.getFastMathFlags().noSignedZeros() && match(Op0, m_PosZeroFP()))
> -    return BinaryOperator::CreateFNegFMF(Op1, &I);
> +  if (I.hasNoSignedZeros()) {
> +    // Subtraction from -0.0 is the canonical form of fneg.
> +    // fsub nsz 0, X ==> fsub nsz -0.0, X
> +    if (match(Op0, m_PosZeroFP()))
> +      return BinaryOperator::CreateFNegFMF(Op1, &I);
> +
> +    // With no-signed-zeros: -(X - Y) --> Y - X
> +    Value *X, *Y;
> +    if (match(Op0, m_NegZeroFP()) && match(Op1, m_FSub(m_Value(X), m_Value(Y))))
> +      return BinaryOperator::CreateFSubFMF(Y, X, &I);
> +  }
> 
>   if (isa<Constant>(Op0))
>     if (SelectInst *SI = dyn_cast<SelectInst>(Op1))
> 
> Modified: llvm/trunk/test/Transforms/InstCombine/fsub.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/fsub.ll?rev=329350&r1=329349&r2=329350&view=diff
> ==============================================================================
> --- llvm/trunk/test/Transforms/InstCombine/fsub.ll (original)
> +++ llvm/trunk/test/Transforms/InstCombine/fsub.ll Thu Apr  5 14:37:17 2018
> @@ -19,8 +19,7 @@ define float @test1(float %x, float %y)
> 
> define float @neg_sub(float %x, float %y) {
> ; CHECK-LABEL: @neg_sub(
> -; CHECK-NEXT:    [[T1:%.*]] = fsub float [[X:%.*]], [[Y:%.*]]
> -; CHECK-NEXT:    [[T2:%.*]] = fsub nsz float -0.000000e+00, [[T1]]
> +; CHECK-NEXT:    [[T2:%.*]] = fsub nsz float [[Y:%.*]], [[X:%.*]]
> ; CHECK-NEXT:    ret float [[T2]]
> ;
>   %t1 = fsub float %x, %y
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits