[llvm] 2ccbf3d - [SDAG] fold x * 0.0 at node creation time

[Quentin Colombet via llvm-commits]

Hi Sanjay,

Just double checking this is intended.

This commit is not NFC because the old code used to check also the global options whereas the new one only checks the individual flags.

Cheers,
-Quentin

> On Oct 4, 2020, at 8:35 AM, Sanjay Patel via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> 
> Author: Sanjay Patel
> Date: 2020-10-04T11:31:57-04:00
> New Revision: 2ccbf3dbd5bac9d4fea8b67404b4c6b006d4adbe
> 
> URL: https://github.com/llvm/llvm-project/commit/2ccbf3dbd5bac9d4fea8b67404b4c6b006d4adbe
> DIFF: https://github.com/llvm/llvm-project/commit/2ccbf3dbd5bac9d4fea8b67404b4c6b006d4adbe.diff
> 
> LOG: [SDAG] fold x * 0.0 at node creation time
> 
> In the motivating case from https://llvm.org/PR47517
> we create a node that does not get constant folded
> before getNegatedExpression is attempted from some
> other node, and we crash.
> 
> By moving the fold into SelectionDAG::simplifyFPBinop(),
> we get the constant fold sooner and avoid the problem.
> 
> Added: 
> 
> 
> Modified: 
>    llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
>    llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
>    llvm/test/CodeGen/ARM/softfp-constant-comparison.ll
>    llvm/test/CodeGen/X86/fmul-combines.ll
> 
> Removed: 
> 
> 
> 
> ################################################################################
> diff  --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
> index 0b6aca4ca34c..9df930a6e3ba 100644
> --- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
> +++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
> @@ -13040,13 +13040,6 @@ SDValue DAGCombiner::visitFMUL(SDNode *N) {
>   if (SDValue NewSel = foldBinOpIntoSelect(N))
>     return NewSel;
> 
> -  if ((Options.NoNaNsFPMath && Options.NoSignedZerosFPMath) ||

^^^ Here we check `Options`

> -      (Flags.hasNoNaNs() && Flags.hasNoSignedZeros())) {
> -    // fold (fmul A, 0) -> 0
> -    if (N1CFP && N1CFP->isZero())
> -      return N1;
> -  }
> -
>   if (Options.UnsafeFPMath || Flags.hasAllowReassociation()) {
>     // fmul (fmul X, C1), C2 -> fmul X, C1 * C2
>     if (isConstantFPBuildVectorOrConstantFP(N1) &&
> 
> diff  --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
> index eef467d116b7..62d01fbf96cd 100644
> --- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
> +++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
> @@ -7475,6 +7475,11 @@ SDValue SelectionDAG::simplifyFPBinop(unsigned Opcode, SDValue X, SDValue Y,
>     if (YC->getValueAPF().isExactlyValue(1.0))
>       return X;
> 
> +  // X * 0.0 --> 0.0
> +  if (Opcode == ISD::FMUL && Flags.hasNoNaNs() && Flags.hasNoSignedZeros())

^^^ Here we only check `Flags`

> +    if (YC->getValueAPF().isZero())
> +      return getConstantFP(0.0, SDLoc(Y), Y.getValueType());
> +
>   return SDValue();
> }
> 
> 
> diff  --git a/llvm/test/CodeGen/ARM/softfp-constant-comparison.ll b/llvm/test/CodeGen/ARM/softfp-constant-comparison.ll
> index 0b4e42843cba..e076e75e8066 100644
> --- a/llvm/test/CodeGen/ARM/softfp-constant-comparison.ll
> +++ b/llvm/test/CodeGen/ARM/softfp-constant-comparison.ll
> @@ -9,24 +9,13 @@ target triple = "thumbv7em-arm-none-eabi"
> define hidden void @fn1() nounwind #0 {
> ; CHECK-LABEL: fn1:
> ; CHECK:       @ %bb.0: @ %entry
> -; CHECK-NEXT:    .save {r7, lr}
> -; CHECK-NEXT:    push {r7, lr}
> -; CHECK-NEXT:    vldr d0, .LCPI0_0
> -; CHECK-NEXT:    vmov r0, r1, d0
> -; CHECK-NEXT:    mov r2, r0
> -; CHECK-NEXT:    mov r3, r1
> -; CHECK-NEXT:    bl __aeabi_dcmpeq
> +; CHECK-NEXT:    movs r0, #1
> ; CHECK-NEXT:    cbnz r0, .LBB0_2
> ; CHECK-NEXT:    b .LBB0_1
> ; CHECK-NEXT:  .LBB0_1: @ %land.rhs
> ; CHECK-NEXT:    b .LBB0_2
> ; CHECK-NEXT:  .LBB0_2: @ %land.end
> -; CHECK-NEXT:    pop {r7, pc}
> -; CHECK-NEXT:    .p2align 3
> -; CHECK-NEXT:  @ %bb.3:
> -; CHECK-NEXT:  .LCPI0_0:
> -; CHECK-NEXT:    .long 0 @ double 0
> -; CHECK-NEXT:    .long 0
> +; CHECK-NEXT:    bx lr
> entry:
>   %0 = load i32, i32* @a, align 4
>   %conv = sitofp i32 %0 to double
> 
> diff  --git a/llvm/test/CodeGen/X86/fmul-combines.ll b/llvm/test/CodeGen/X86/fmul-combines.ll
> index 0c8a822e7ff7..f9a3e75c3d78 100644
> --- a/llvm/test/CodeGen/X86/fmul-combines.ll
> +++ b/llvm/test/CodeGen/X86/fmul-combines.ll
> @@ -261,3 +261,22 @@ define <4 x float> @fmul_fneg_fneg_v4f32(<4 x float> %x, <4 x float> %y) {
>   %mul = fmul <4 x float> %x.neg, %y.neg
>   ret <4 x float> %mul
> }
> +
> +; PR47517 - this could crash if we create 'fmul x, 0.0' nodes
> +; that do not constant fold in a particular order.
> +
> +define float @getNegatedExpression_crash(float* %p) {
> +; CHECK-LABEL: getNegatedExpression_crash:
> +; CHECK:       # %bb.0:
> +; CHECK-NEXT:    movl $0, (%rdi)
> +; CHECK-NEXT:    xorps %xmm0, %xmm0
> +; CHECK-NEXT:    retq
> +  store float 0.0, float* %p, align 1
> +  %real = load float, float* %p, align 1
> +  %r2 = fmul fast float %real, %real
> +  %t1 = fmul fast float %real, 42.0
> +  %t2 = fmul fast float %real, %t1
> +  %mul_ac56 = fmul fast float %t2, %t1
> +  %mul_ac72 = fmul fast float %r2, %mul_ac56
> +  ret float %mul_ac72
> +}
> 
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits