<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="">okay, will test this, thanks!<div class=""><br class=""></div><div class="">—escha<br class=""><div><br class=""><blockquote type="cite" class=""><div class="">On Apr 12, 2018, at 3:55 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="">Patch that might fix the problem proposed here:<br class=""><a href="https://reviews.llvm.org/D45598" rel="noreferrer" target="_blank" class="">https://reviews.llvm.org/<wbr class="">D45598</a><br class=""></div><div class="gmail_extra"><br class=""><div class="gmail_quote">On Thu, Apr 12, 2018 at 1:00 PM, Sanjay Patel <span dir="ltr" class=""><<a href="mailto:spatel@rotateright.com" target="_blank" class="">spatel@rotateright.com</a>></span> wrote:<br class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr" class=""><div class=""><div class="">Hmm...so this one is different than r329350, right?<br class=""><br class=""></div>In this case, we really shouldn't canonicalize to a form with more instructions. Add a DAGCombine that does the transform if fpext/fptrunc are free?<br class=""><br class=""></div>Let me know if I should revert in the meantime.<br class=""></div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br class=""><div class="gmail_quote">On Thu, Apr 12, 2018 at 11:57 AM, <span dir="ltr" class=""><<a href="mailto:escha@apple.com" target="_blank" class="">escha@apple.com</a>></span> wrote:<br class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">hi! this patch caused a 0.14% instruction count regression on our test suite. it is likely quite bad for us because fpext/fptrunc are free.<br class="">
<span class="m_2751570223863815656HOEnZb"><font color="#888888" class=""><br class="">
—escha<br class="">
</font></span><div class="m_2751570223863815656HOEnZb"><div class="m_2751570223863815656h5"><br class="">
> On Apr 11, 2018, at 8:57 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="">
> Author: spatel<br class="">
> Date: Wed Apr 11 08:57:18 2018<br class="">
> New Revision: 329821<br class="">
> <br class="">
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=329821&view=rev" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-pr<wbr class="">oject?rev=329821&view=rev</a><br class="">
> Log:<br class="">
> [InstCombine] limit X - (cast(-Y) --> X + cast(Y) with hasOneUse()<br class="">
> <br class="">
> Modified:<br class="">
> llvm/trunk/lib/Transforms/Inst<wbr class="">Combine/InstCombineAddSub.cpp<br class="">
> llvm/trunk/test/Transforms/Ins<wbr class="">tCombine/fsub.ll<br class="">
> <br class="">
> Modified: llvm/trunk/lib/Transforms/Inst<wbr class="">Combine/InstCombineAddSub.cpp<br class="">
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineAddSub.cpp?rev=329821&r1=329820&r2=329821&view=diff" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-pr<wbr class="">oject/llvm/trunk/lib/Transform<wbr class="">s/InstCombine/InstCombineAddSu<wbr class="">b.cpp?rev=329821&r1=329820&r2=<wbr class="">329821&view=diff</a><br class="">
> ==============================<wbr class="">==============================<wbr class="">==================<br class="">
> --- llvm/trunk/lib/Transforms/Inst<wbr class="">Combine/InstCombineAddSub.cpp (original)<br class="">
> +++ llvm/trunk/lib/Transforms/Inst<wbr class="">Combine/InstCombineAddSub.cpp Wed Apr 11 08:57:18 2018<br class="">
> @@ -1731,16 +1731,16 @@ Instruction *InstCombiner::visitFSub(Bin<br class="">
> if (match(Op1, m_FNeg(m_Value(Y))))<br class="">
> return BinaryOperator::CreateFAddFMF(<wbr class="">Op0, Y, &I);<br class="">
> <br class="">
> - if (FPTruncInst *FPTI = dyn_cast<FPTruncInst>(Op1)) {<br class="">
> - if (Value *V = dyn_castFNegVal(FPTI->getOpera<wbr class="">nd(0))) {<br class="">
> - Value *NewTrunc = Builder.CreateFPTrunc(V, I.getType());<br class="">
> - return BinaryOperator::CreateFAddFMF(<wbr class="">Op0, NewTrunc, &I);<br class="">
> - }<br class="">
> - } else if (FPExtInst *FPEI = dyn_cast<FPExtInst>(Op1)) {<br class="">
> - if (Value *V = dyn_castFNegVal(FPEI->getOpera<wbr class="">nd(0))) {<br class="">
> - Value *NewExt = Builder.CreateFPExt(V, I.getType());<br class="">
> - return BinaryOperator::CreateFAddFMF(<wbr class="">Op0, NewExt, &I);<br class="">
> - }<br class="">
> + // Similar to above, but look through a cast of the negated value:<br class="">
> + // X - (fptrunc(-Y)) --> X + fptrunc(Y)<br class="">
> + if (match(Op1, m_OneUse(m_FPTrunc(m_FNeg(m_Va<wbr class="">lue(Y)))))) {<br class="">
> + Value *TruncY = Builder.CreateFPTrunc(Y, I.getType());<br class="">
> + return BinaryOperator::CreateFAddFMF(<wbr class="">Op0, TruncY, &I);<br class="">
> + }<br class="">
> + // X - (fpext(-Y)) --> X + fpext(Y)<br class="">
> + if (match(Op1, m_OneUse(m_FPExt(m_FNeg(m_Valu<wbr class="">e(Y)))))) {<br class="">
> + Value *ExtY = Builder.CreateFPExt(Y, I.getType());<br class="">
> + return BinaryOperator::CreateFAddFMF(<wbr class="">Op0, ExtY, &I);<br class="">
> }<br class="">
> <br class="">
> // Handle specials cases for FSub with selects feeding the operation<br class="">
> <br class="">
> Modified: llvm/trunk/test/Transforms/Ins<wbr class="">tCombine/fsub.ll<br class="">
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/fsub.ll?rev=329821&r1=329820&r2=329821&view=diff" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-pr<wbr class="">oject/llvm/trunk/test/Transfor<wbr class="">ms/InstCombine/fsub.ll?rev=<wbr class="">329821&r1=329820&r2=329821&<wbr class="">view=diff</a><br class="">
> ==============================<wbr class="">==============================<wbr class="">==================<br class="">
> --- llvm/trunk/test/Transforms/Ins<wbr class="">tCombine/fsub.ll (original)<br class="">
> +++ llvm/trunk/test/Transforms/Ins<wbr class="">tCombine/fsub.ll Wed Apr 11 08:57:18 2018<br class="">
> @@ -189,14 +189,13 @@ define double @neg_ext_op1_fast(float %a<br class="">
> ret double %t3<br class="">
> }<br class="">
> <br class="">
> -; FIXME: Extra use should prevent the transform.<br class="">
> +; Extra use should prevent the transform.<br class="">
> <br class="">
> define float @neg_ext_op1_extra_use(half %a, float %b) {<br class="">
> ; CHECK-LABEL: @neg_ext_op1_extra_use(<br class="">
> ; CHECK-NEXT: [[T1:%.*]] = fsub half 0xH8000, [[A:%.*]]<br class="">
> ; CHECK-NEXT: [[T2:%.*]] = fpext half [[T1]] to float<br class="">
> -; CHECK-NEXT: [[TMP1:%.*]] = fpext half [[A]] to float<br class="">
> -; CHECK-NEXT: [[T3:%.*]] = fadd float [[TMP1]], [[B:%.*]]<br class="">
> +; CHECK-NEXT: [[T3:%.*]] = fsub float [[B:%.*]], [[T2]]<br class="">
> ; CHECK-NEXT: call void @use(float [[T2]])<br class="">
> ; CHECK-NEXT: ret float [[T3]]<br class="">
> ;<br class="">
> @@ -207,8 +206,8 @@ define float @neg_ext_op1_extra_use(half<br class="">
> ret float %t3<br class="">
> }<br class="">
> <br class="">
> -; One-use fptrunc is always hoisted above fneg, so the corresponding <br class="">
> -; multi-use bug for fptrunc isn't visible with a fold starting from <br class="">
> +; One-use fptrunc is always hoisted above fneg, so the corresponding<br class="">
> +; multi-use bug for fptrunc isn't visible with a fold starting from<br class="">
> ; the last fsub.<br class="">
> <br class="">
> define float @neg_trunc_op1_extra_use(doubl<wbr class="">e %a, float %b) {<br class="">
> @@ -226,15 +225,13 @@ define float @neg_trunc_op1_extra_use(do<br class="">
> ret float %t3<br class="">
> }<br class="">
> <br class="">
> -; FIXME: But the bug is visible when both preceding values have other uses.<br class="">
> ; Extra uses should prevent the transform.<br class="">
> <br class="">
> define float @neg_trunc_op1_extra_uses(doub<wbr class="">le %a, float %b) {<br class="">
> ; CHECK-LABEL: @neg_trunc_op1_extra_uses(<br class="">
> ; CHECK-NEXT: [[T1:%.*]] = fsub double -0.000000e+00, [[A:%.*]]<br class="">
> ; CHECK-NEXT: [[T2:%.*]] = fptrunc double [[T1]] to float<br class="">
> -; CHECK-NEXT: [[TMP1:%.*]] = fptrunc double [[A]] to float<br class="">
> -; CHECK-NEXT: [[T3:%.*]] = fadd float [[TMP1]], [[B:%.*]]<br class="">
> +; CHECK-NEXT: [[T3:%.*]] = fsub float [[B:%.*]], [[T2]]<br class="">
> ; CHECK-NEXT: call void @use2(float [[T2]], double [[T1]])<br class="">
> ; CHECK-NEXT: ret float [[T3]]<br class="">
> ;<br class="">
> <br class="">
> <br class="">
> ______________________________<wbr 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="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank" class="">http://lists.llvm.org/cgi-bin/<wbr class="">mailman/listinfo/llvm-commits</a><br class="">
<br class="">
</div></div></blockquote></div><br class=""></div>
</div></div></blockquote></div><br class=""></div>
</div></blockquote></div><br class=""></div></body></html>