[llvm] r329821 - [InstCombine] limit X - (cast(-Y) --> X + cast(Y) with hasOneUse()

via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 13 11:57:00 PDT 2018


this doesn’t seem to help at all, even if I add isfpextfree to our targets. i suspect we’re losing out on a transformation done during the IR stage (e.g. reassociation) as a result of this. i’m going to look into it further.

—escha

> On Apr 13, 2018, at 9:40 AM, via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> okay, will test this, thanks!
> 
> —escha
> 
>> On Apr 12, 2018, at 3:55 PM, Sanjay Patel <spatel at rotateright.com <mailto:spatel at rotateright.com>> wrote:
>> 
>> Patch that might fix the problem proposed here:
>> https://reviews.llvm.org/D45598 <https://reviews.llvm.org/D45598>
>> 
>> On Thu, Apr 12, 2018 at 1:00 PM, Sanjay Patel <spatel at rotateright.com <mailto:spatel at rotateright.com>> wrote:
>> Hmm...so this one is different than r329350, right?
>> 
>> 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?
>> 
>> Let me know if I should revert in the meantime.
>> 
>> On Thu, Apr 12, 2018 at 11:57 AM, <escha at apple.com <mailto:escha at apple.com>> wrote:
>> 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.
>> 
>> —escha
>> 
>> > On Apr 11, 2018, at 8:57 AM, Sanjay Patel via llvm-commits <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> wrote:
>> > 
>> > Author: spatel
>> > Date: Wed Apr 11 08:57:18 2018
>> > New Revision: 329821
>> > 
>> > URL: http://llvm.org/viewvc/llvm-project?rev=329821&view=rev <http://llvm.org/viewvc/llvm-project?rev=329821&view=rev>
>> > Log:
>> > [InstCombine] limit X - (cast(-Y) --> X + cast(Y) with hasOneUse()
>> > 
>> > Modified:
>> >    llvm/trunk/lib/Transforms/InstCombine/InstCombineAddSub.cpp
>> >    llvm/trunk/test/Transforms/InstCombine/fsub.ll
>> > 
>> > Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineAddSub.cpp
>> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineAddSub.cpp?rev=329821&r1=329820&r2=329821&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineAddSub.cpp?rev=329821&r1=329820&r2=329821&view=diff>
>> > ==============================================================================
>> > --- llvm/trunk/lib/Transforms/InstCombine/InstCombineAddSub.cpp (original)
>> > +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineAddSub.cpp Wed Apr 11 08:57:18 2018
>> > @@ -1731,16 +1731,16 @@ Instruction *InstCombiner::visitFSub(Bin
>> >   if (match(Op1, m_FNeg(m_Value(Y))))
>> >     return BinaryOperator::CreateFAddFMF(Op0, Y, &I);
>> > 
>> > -  if (FPTruncInst *FPTI = dyn_cast<FPTruncInst>(Op1)) {
>> > -    if (Value *V = dyn_castFNegVal(FPTI->getOperand(0))) {
>> > -      Value *NewTrunc = Builder.CreateFPTrunc(V, I.getType());
>> > -      return BinaryOperator::CreateFAddFMF(Op0, NewTrunc, &I);
>> > -    }
>> > -  } else if (FPExtInst *FPEI = dyn_cast<FPExtInst>(Op1)) {
>> > -    if (Value *V = dyn_castFNegVal(FPEI->getOperand(0))) {
>> > -      Value *NewExt = Builder.CreateFPExt(V, I.getType());
>> > -      return BinaryOperator::CreateFAddFMF(Op0, NewExt, &I);
>> > -    }
>> > +  // Similar to above, but look through a cast of the negated value:
>> > +  // X - (fptrunc(-Y)) --> X + fptrunc(Y)
>> > +  if (match(Op1, m_OneUse(m_FPTrunc(m_FNeg(m_Value(Y)))))) {
>> > +    Value *TruncY = Builder.CreateFPTrunc(Y, I.getType());
>> > +    return BinaryOperator::CreateFAddFMF(Op0, TruncY, &I);
>> > +  }
>> > +  // X - (fpext(-Y)) --> X + fpext(Y)
>> > +  if (match(Op1, m_OneUse(m_FPExt(m_FNeg(m_Value(Y)))))) {
>> > +    Value *ExtY = Builder.CreateFPExt(Y, I.getType());
>> > +    return BinaryOperator::CreateFAddFMF(Op0, ExtY, &I);
>> >   }
>> > 
>> >   // Handle specials cases for FSub with selects feeding the operation
>> > 
>> > Modified: llvm/trunk/test/Transforms/InstCombine/fsub.ll
>> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/fsub.ll?rev=329821&r1=329820&r2=329821&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/fsub.ll?rev=329821&r1=329820&r2=329821&view=diff>
>> > ==============================================================================
>> > --- llvm/trunk/test/Transforms/InstCombine/fsub.ll (original)
>> > +++ llvm/trunk/test/Transforms/InstCombine/fsub.ll Wed Apr 11 08:57:18 2018
>> > @@ -189,14 +189,13 @@ define double @neg_ext_op1_fast(float %a
>> >   ret double %t3
>> > }
>> > 
>> > -; FIXME: Extra use should prevent the transform.
>> > +; Extra use should prevent the transform.
>> > 
>> > define float @neg_ext_op1_extra_use(half %a, float %b) {
>> > ; CHECK-LABEL: @neg_ext_op1_extra_use(
>> > ; CHECK-NEXT:    [[T1:%.*]] = fsub half 0xH8000, [[A:%.*]]
>> > ; CHECK-NEXT:    [[T2:%.*]] = fpext half [[T1]] to float
>> > -; CHECK-NEXT:    [[TMP1:%.*]] = fpext half [[A]] to float
>> > -; CHECK-NEXT:    [[T3:%.*]] = fadd float [[TMP1]], [[B:%.*]]
>> > +; CHECK-NEXT:    [[T3:%.*]] = fsub float [[B:%.*]], [[T2]]
>> > ; CHECK-NEXT:    call void @use(float [[T2]])
>> > ; CHECK-NEXT:    ret float [[T3]]
>> > ;
>> > @@ -207,8 +206,8 @@ define float @neg_ext_op1_extra_use(half
>> >   ret float %t3
>> > }
>> > 
>> > -; One-use fptrunc is always hoisted above fneg, so the corresponding 
>> > -; multi-use bug for fptrunc isn't visible with a fold starting from 
>> > +; One-use fptrunc is always hoisted above fneg, so the corresponding
>> > +; multi-use bug for fptrunc isn't visible with a fold starting from
>> > ; the last fsub.
>> > 
>> > define float @neg_trunc_op1_extra_use(double %a, float %b) {
>> > @@ -226,15 +225,13 @@ define float @neg_trunc_op1_extra_use(do
>> >   ret float %t3
>> > }
>> > 
>> > -; FIXME: But the bug is visible when both preceding values have other uses.
>> > ; Extra uses should prevent the transform.
>> > 
>> > define float @neg_trunc_op1_extra_uses(double %a, float %b) {
>> > ; CHECK-LABEL: @neg_trunc_op1_extra_uses(
>> > ; CHECK-NEXT:    [[T1:%.*]] = fsub double -0.000000e+00, [[A:%.*]]
>> > ; CHECK-NEXT:    [[T2:%.*]] = fptrunc double [[T1]] to float
>> > -; CHECK-NEXT:    [[TMP1:%.*]] = fptrunc double [[A]] to float
>> > -; CHECK-NEXT:    [[T3:%.*]] = fadd float [[TMP1]], [[B:%.*]]
>> > +; CHECK-NEXT:    [[T3:%.*]] = fsub float [[B:%.*]], [[T2]]
>> > ; CHECK-NEXT:    call void @use2(float [[T2]], double [[T1]])
>> > ; CHECK-NEXT:    ret float [[T3]]
>> > ;
>> > 
>> > 
>> > _______________________________________________
>> > llvm-commits mailing list
>> > llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits <http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits>
>> 
>> 
>> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://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/20180413/609404b1/attachment-0001.html>


More information about the llvm-commits mailing list