<html><head><meta http-equiv="Content-Type" content="text/html; charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Oct 6, 2020, at 3:07 PM, Sanjay Patel <<a href="mailto:spatel@rotateright.com" class="">spatel@rotateright.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class=""><div class="">Correct - this patch is not NFC for 2 reasons:</div><div class="">1. It constant folds cases that might have escaped before (as seen in the ARM test).</div><div class="">2. It uses the flags exclusively.</div><div class=""><br class=""></div><div class="">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.<br class=""></div></div></div></blockquote><div><br class=""></div><div>I assume that’s true too.</div><div><br class=""></div><div>Thanks for the confirmation!</div><br class=""><blockquote type="cite" class=""><div class=""><br class=""><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Oct 6, 2020 at 5:11 PM Quentin Colombet <<a href="mailto:qcolombet@apple.com" class="">qcolombet@apple.com</a>> wrote:<br class=""></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Sanjay,<br class="">
<br class="">
Just double checking this is intended.<br class="">
<br class="">
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.<br class="">
<br class="">
Cheers,<br class="">
-Quentin<br class="">
<br class="">
> On Oct 4, 2020, at 8:35 AM, Sanjay Patel via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank" class="">llvm-commits@lists.llvm.org</a>> wrote:<br class="">
> <br class="">
> <br class="">
> Author: Sanjay Patel<br class="">
> Date: 2020-10-04T11:31:57-04:00<br class="">
> New Revision: 2ccbf3dbd5bac9d4fea8b67404b4c6b006d4adbe<br class="">
> <br class="">
> URL: <a href="https://github.com/llvm/llvm-project/commit/2ccbf3dbd5bac9d4fea8b67404b4c6b006d4adbe" rel="noreferrer" target="_blank" class="">https://github.com/llvm/llvm-project/commit/2ccbf3dbd5bac9d4fea8b67404b4c6b006d4adbe</a><br class="">
> DIFF: <a href="https://github.com/llvm/llvm-project/commit/2ccbf3dbd5bac9d4fea8b67404b4c6b006d4adbe.diff" rel="noreferrer" target="_blank" class="">https://github.com/llvm/llvm-project/commit/2ccbf3dbd5bac9d4fea8b67404b4c6b006d4adbe.diff</a><br class="">
> <br class="">
> LOG: [SDAG] fold x * 0.0 at node creation time<br class="">
> <br class="">
> In the motivating case from <a href="https://llvm.org/PR47517" rel="noreferrer" target="_blank" class="">https://llvm.org/PR47517</a><br class="">
> we create a node that does not get constant folded<br class="">
> before getNegatedExpression is attempted from some<br class="">
> other node, and we crash.<br class="">
> <br class="">
> By moving the fold into SelectionDAG::simplifyFPBinop(),<br class="">
> we get the constant fold sooner and avoid the problem.<br class="">
> <br class="">
> Added: <br class="">
> <br class="">
> <br class="">
> Modified: <br class="">
> llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp<br class="">
> llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp<br class="">
> llvm/test/CodeGen/ARM/softfp-constant-comparison.ll<br class="">
> llvm/test/CodeGen/X86/fmul-combines.ll<br class="">
> <br class="">
> Removed: <br class="">
> <br class="">
> <br class="">
> <br class="">
> ################################################################################<br class="">
> diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp<br class="">
> index 0b6aca4ca34c..9df930a6e3ba 100644<br class="">
> --- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp<br class="">
> +++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp<br class="">
> @@ -13040,13 +13040,6 @@ SDValue DAGCombiner::visitFMUL(SDNode *N) {<br class="">
> if (SDValue NewSel = foldBinOpIntoSelect(N))<br class="">
> return NewSel;<br class="">
> <br class="">
> - if ((Options.NoNaNsFPMath && Options.NoSignedZerosFPMath) ||<br class="">
<br class="">
^^^ Here we check `Options`<br class="">
<br class="">
> - (Flags.hasNoNaNs() && Flags.hasNoSignedZeros())) {<br class="">
> - // fold (fmul A, 0) -> 0<br class="">
> - if (N1CFP && N1CFP->isZero())<br class="">
> - return N1;<br class="">
> - }<br class="">
> -<br class="">
> if (Options.UnsafeFPMath || Flags.hasAllowReassociation()) {<br class="">
> // fmul (fmul X, C1), C2 -> fmul X, C1 * C2<br class="">
> if (isConstantFPBuildVectorOrConstantFP(N1) &&<br class="">
> <br class="">
> diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp<br class="">
> index eef467d116b7..62d01fbf96cd 100644<br class="">
> --- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp<br class="">
> +++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp<br class="">
> @@ -7475,6 +7475,11 @@ SDValue SelectionDAG::simplifyFPBinop(unsigned Opcode, SDValue X, SDValue Y,<br class="">
> if (YC->getValueAPF().isExactlyValue(1.0))<br class="">
> return X;<br class="">
> <br class="">
> + // X * 0.0 --> 0.0<br class="">
> + if (Opcode == ISD::FMUL && Flags.hasNoNaNs() && Flags.hasNoSignedZeros())<br class="">
<br class="">
^^^ Here we only check `Flags`<br class="">
<br class="">
> + if (YC->getValueAPF().isZero())<br class="">
> + return getConstantFP(0.0, SDLoc(Y), Y.getValueType());<br class="">
> +<br class="">
> return SDValue();<br class="">
> }<br class="">
> <br class="">
> <br class="">
> diff --git a/llvm/test/CodeGen/ARM/softfp-constant-comparison.ll b/llvm/test/CodeGen/ARM/softfp-constant-comparison.ll<br class="">
> index 0b4e42843cba..e076e75e8066 100644<br class="">
> --- a/llvm/test/CodeGen/ARM/softfp-constant-comparison.ll<br class="">
> +++ b/llvm/test/CodeGen/ARM/softfp-constant-comparison.ll<br class="">
> @@ -9,24 +9,13 @@ target triple = "thumbv7em-arm-none-eabi"<br class="">
> define hidden void @fn1() nounwind #0 {<br class="">
> ; CHECK-LABEL: fn1:<br class="">
> ; CHECK: @ %bb.0: @ %entry<br class="">
> -; CHECK-NEXT: .save {r7, lr}<br class="">
> -; CHECK-NEXT: push {r7, lr}<br class="">
> -; CHECK-NEXT: vldr d0, .LCPI0_0<br class="">
> -; CHECK-NEXT: vmov r0, r1, d0<br class="">
> -; CHECK-NEXT: mov r2, r0<br class="">
> -; CHECK-NEXT: mov r3, r1<br class="">
> -; CHECK-NEXT: bl __aeabi_dcmpeq<br class="">
> +; CHECK-NEXT: movs r0, #1<br class="">
> ; CHECK-NEXT: cbnz r0, .LBB0_2<br class="">
> ; CHECK-NEXT: b .LBB0_1<br class="">
> ; CHECK-NEXT: .LBB0_1: @ %land.rhs<br class="">
> ; CHECK-NEXT: b .LBB0_2<br class="">
> ; CHECK-NEXT: .LBB0_2: @ %land.end<br class="">
> -; CHECK-NEXT: pop {r7, pc}<br class="">
> -; CHECK-NEXT: .p2align 3<br class="">
> -; CHECK-NEXT: @ %bb.3:<br class="">
> -; CHECK-NEXT: .LCPI0_0:<br class="">
> -; CHECK-NEXT: .long 0 @ double 0<br class="">
> -; CHECK-NEXT: .long 0<br class="">
> +; CHECK-NEXT: bx lr<br class="">
> entry:<br class="">
> %0 = load i32, i32* @a, align 4<br class="">
> %conv = sitofp i32 %0 to double<br class="">
> <br class="">
> diff --git a/llvm/test/CodeGen/X86/fmul-combines.ll b/llvm/test/CodeGen/X86/fmul-combines.ll<br class="">
> index 0c8a822e7ff7..f9a3e75c3d78 100644<br class="">
> --- a/llvm/test/CodeGen/X86/fmul-combines.ll<br class="">
> +++ b/llvm/test/CodeGen/X86/fmul-combines.ll<br class="">
> @@ -261,3 +261,22 @@ define <4 x float> @fmul_fneg_fneg_v4f32(<4 x float> %x, <4 x float> %y) {<br class="">
> %mul = fmul <4 x float> %x.neg, %y.neg<br class="">
> ret <4 x float> %mul<br class="">
> }<br class="">
> +<br class="">
> +; PR47517 - this could crash if we create 'fmul x, 0.0' nodes<br class="">
> +; that do not constant fold in a particular order.<br class="">
> +<br class="">
> +define float @getNegatedExpression_crash(float* %p) {<br class="">
> +; CHECK-LABEL: getNegatedExpression_crash:<br class="">
> +; CHECK: # %bb.0:<br class="">
> +; CHECK-NEXT: movl $0, (%rdi)<br class="">
> +; CHECK-NEXT: xorps %xmm0, %xmm0<br class="">
> +; CHECK-NEXT: retq<br class="">
> + store float 0.0, float* %p, align 1<br class="">
> + %real = load float, float* %p, align 1<br class="">
> + %r2 = fmul fast float %real, %real<br class="">
> + %t1 = fmul fast float %real, 42.0<br class="">
> + %t2 = fmul fast float %real, %t1<br class="">
> + %mul_ac56 = fmul fast float %t2, %t1<br class="">
> + %mul_ac72 = fmul fast float %r2, %mul_ac56<br class="">
> + ret float %mul_ac72<br class="">
> +}<br class="">
> <br class="">
> <br class="">
> <br class="">
> _______________________________________________<br class="">
> llvm-commits mailing list<br class="">
> <a href="mailto:llvm-commits@lists.llvm.org" target="_blank" class="">llvm-commits@lists.llvm.org</a><br class="">
> <a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank" class="">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br class="">
<br class="">
</blockquote></div>
</div></blockquote></div><br class=""></body></html>