[llvm] r339176 - [InstSimplify] fold fsub+fadd with common operand

[Roman Tereshin via llvm-commits]

Hi Sanjay,

+ Michael


I'm not sure it's enough to check for nsz and reassoc flags here.

I think reassoc let's you do

Y + (X - Y)  -->   (Y - Y) + X

But w/o knowing that Y is non a NaN and not an infinity (of any sign) we can't tell if the result is a NaN or X, I believe.
The check could be done by checking just the nnan flag on the top level op.

Shouldn't we check for the nnan flag here as well while doing this?


Another question here is that we check here the top-level reassoc flag only. I'm not sure if it's enough either.

Meaning, I think it's possible that while

(Y + (X - Y)<reassoc>)<reassoc>  -->   ((Y - Y)<reassoc> + X)<reassoc>

is perfectly legal

(Y + (X - Y))<reassoc>  -->  (Y - Y) + X

is not allowed.

What do you think?


Thanks,
Roman

> On Aug 7, 2018, at 1:32 PM, Sanjay Patel via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> Author: spatel
> Date: Tue Aug  7 13:32:55 2018
> New Revision: 339176
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=339176&view=rev
> Log:
> [InstSimplify] fold fsub+fadd with common operand
> 
> Modified:
>    llvm/trunk/lib/Analysis/InstructionSimplify.cpp
>    llvm/trunk/test/Transforms/InstSimplify/floating-point-arithmetic.ll
> 
> Modified: llvm/trunk/lib/Analysis/InstructionSimplify.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/InstructionSimplify.cpp?rev=339176&r1=339175&r2=339176&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Analysis/InstructionSimplify.cpp (original)
> +++ llvm/trunk/lib/Analysis/InstructionSimplify.cpp Tue Aug  7 13:32:55 2018
> @@ -4359,6 +4359,14 @@ static Value *SimplifyFAddInst(Value *Op
>                        match(Op1, m_FSub(m_AnyZeroFP(), m_Specific(Op0)))))
>     return ConstantFP::getNullValue(Op0->getType());
> 
> +  // (X - Y) + Y --> X
> +  // Y + (X - Y) --> X
> +  Value *X;
> +  if (FMF.noSignedZeros() && FMF.allowReassoc() &&
> +      (match(Op0, m_FSub(m_Value(X), m_Specific(Op1))) ||
> +       match(Op1, m_FSub(m_Value(X), m_Specific(Op0)))))
> +    return X;
> +
>   return nullptr;
> }
> 
> 
> Modified: llvm/trunk/test/Transforms/InstSimplify/floating-point-arithmetic.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstSimplify/floating-point-arithmetic.ll?rev=339176&r1=339175&r2=339176&view=diff
> ==============================================================================
> --- llvm/trunk/test/Transforms/InstSimplify/floating-point-arithmetic.ll (original)
> +++ llvm/trunk/test/Transforms/InstSimplify/floating-point-arithmetic.ll Tue Aug  7 13:32:55 2018
> @@ -849,9 +849,7 @@ define float @fadd_fsub_common_op_wrong_
> 
> define <2 x float> @fsub_fadd_common_op_vec(<2 x float> %x, <2 x float> %y) {
> ; CHECK-LABEL: @fsub_fadd_common_op_vec(
> -; CHECK-NEXT:    [[S:%.*]] = fsub <2 x float> [[X:%.*]], [[Y:%.*]]
> -; CHECK-NEXT:    [[R:%.*]] = fadd reassoc nsz <2 x float> [[Y]], [[S]]
> -; CHECK-NEXT:    ret <2 x float> [[R]]
> +; CHECK-NEXT:    ret <2 x float> [[X:%.*]]
> ;
>   %s = fsub <2 x float> %x, %y
>   %r = fadd reassoc nsz <2 x float> %y, %s
> @@ -862,9 +860,7 @@ define <2 x float> @fsub_fadd_common_op_
> 
> define float @fsub_fadd_common_op_commute(float %x, float %y) {
> ; CHECK-LABEL: @fsub_fadd_common_op_commute(
> -; CHECK-NEXT:    [[S:%.*]] = fsub float [[X:%.*]], [[Y:%.*]]
> -; CHECK-NEXT:    [[R:%.*]] = fadd reassoc nsz float [[S]], [[Y]]
> -; CHECK-NEXT:    ret float [[R]]
> +; CHECK-NEXT:    ret float [[X:%.*]]
> ;
>   %s = fsub float %x, %y
>   %r = fadd reassoc nsz float %s, %y
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits