[llvm] 2ccbf3d - [SDAG] fold x * 0.0 at node creation time
Quentin Colombet via llvm-commits
llvm-commits at lists.llvm.org
Tue Oct 6 15:39:40 PDT 2020
> On Oct 6, 2020, at 3:07 PM, Sanjay Patel <spatel at rotateright.com> wrote:
>
> Correct - this patch is not NFC for 2 reasons:
> 1. It constant folds cases that might have escaped before (as seen in the ARM test).
> 2. It uses the flags exclusively.
>
> I assumed that the node-level flags are created and propagated correctly now, so the target option is effectively deprecated. If that's not correct, let me know.
I assume that’s true too.
Thanks for the confirmation!
>
> On Tue, Oct 6, 2020 at 5:11 PM Quentin Colombet <qcolombet at apple.com <mailto:qcolombet at apple.com>> wrote:
> 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 <mailto: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 <https://github.com/llvm/llvm-project/commit/2ccbf3dbd5bac9d4fea8b67404b4c6b006d4adbe>
> > DIFF: 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 <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 <mailto:llvm-commits at lists.llvm.org>
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits <https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20201006/92d1ef35/attachment.html>
More information about the llvm-commits
mailing list