I remember why I didn't implement this rule in Instcombine. It add one instruction. So, this xform should be driven by a redundancy eliminator if you care code size. On 8/8/13 10:13 AM, Shuxin Yang wrote:> I did few transformation in Instruction *InstCombiner::visitFDiv() in > an attempt to remove some divs. > I may miss this case. If you need to implement this rule, it is > better done in Instcombine than in DAG combine. > Doing such xform early expose the redundancy of 1/y, which have > positive impact to neighboring code, > while DAG combine is bit blind. > > You should be very careful, reciprocal is very very very imprecise > transformation. Sometimes you will see big different > with and without this xform. > > On 8/8/13 9:25 AM, Chad Rosier wrote: >> I would like to transform X/Y -> X*1/Y. Specifically, I would like to >> convert: >> >> define void @t1a(double %a, double %b, double %d) { >> entry: >> %div = fdiv fast double %a, %d >> %div1 = fdiv fast double %b, %d >> %call = tail call i32 @foo(double %div, double %div1) >> ret void >> } >> >> to: >> >> define void @t1b(double %a, double %b, double %d) { >> entry: >> %div = fdiv fast double 1.000000e+00, %d >> %mul = fmul fast double %div, %a >> %mul1 = fmul fast double %div, %b >> %call = tail call i32 @foo(double %mul, double %mul1) >> ret void >> } >> >> Is such a transformation best done as a (target-specific) DAG combine? >> >> A similar instcombine already exists for the X/C->X*1/C case (see the >> CvtFDivConstToReciprocal function in InstCombineMlDivRem.cpp), but I >> don't believe the above can be done as an instcombine as it creates a >> new instruction (in addition to replacing the original). Also, I >> only want to perform the transformation if there are multiple uses of >> 1/Y (like in my test case). Otherwise, the transformation replaces a >> fdiv with a fdiv+fmul pair, which I doubt would be profitable. >> >> FWIW, I'm also pretty sure this combine requires -fast-math. >> >> Can someone point me in the right direction? >> >> Thanks, >> Chad >> >> >> _______________________________________________ >> LLVM Developers mailing list >> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20130808/fb576dd0/attachment.html>
Yes, I believe I commented on that concern in my original email (Perhaps not for this reason, however). Ben pointed out that GVN should be able to clean things up if there are multiple instances of 1/Y. On Thu, Aug 8, 2013 at 1:21 PM, Shuxin Yang <shuxin.llvm at gmail.com> wrote:> I remember why I didn't implement this rule in Instcombine. It add one > instruction. So, > this xform should be driven by a redundancy eliminator if you care code > size. > > On 8/8/13 10:13 AM, Shuxin Yang wrote: > > I did few transformation in Instruction *InstCombiner::visitFDiv() in an > attempt to remove some divs. > I may miss this case. If you need to implement this rule, it is better > done in Instcombine than in DAG combine. > Doing such xform early expose the redundancy of 1/y, which have positive > impact to neighboring code, > while DAG combine is bit blind. > > You should be very careful, reciprocal is very very very imprecise > transformation. Sometimes you will see big different > with and without this xform. > > On 8/8/13 9:25 AM, Chad Rosier wrote: > > I would like to transform X/Y -> X*1/Y. Specifically, I would like to > convert: > > define void @t1a(double %a, double %b, double %d) { > entry: > %div = fdiv fast double %a, %d > %div1 = fdiv fast double %b, %d > %call = tail call i32 @foo(double %div, double %div1) > ret void > } > > to: > > define void @t1b(double %a, double %b, double %d) { > entry: > %div = fdiv fast double 1.000000e+00, %d > %mul = fmul fast double %div, %a > %mul1 = fmul fast double %div, %b > %call = tail call i32 @foo(double %mul, double %mul1) > ret void > } > > Is such a transformation best done as a (target-specific) DAG combine? > > A similar instcombine already exists for the X/C->X*1/C case (see the > CvtFDivConstToReciprocal function in InstCombineMlDivRem.cpp), but I don't > believe the above can be done as an instcombine as it creates a new > instruction (in addition to replacing the original). Also, I only want to > perform the transformation if there are multiple uses of 1/Y (like in my > test case). Otherwise, the transformation replaces a fdiv with a fdiv+fmul > pair, which I doubt would be profitable. > > FWIW, I'm also pretty sure this combine requires -fast-math. > > Can someone point me in the right direction? > > Thanks, > Chad > > > _______________________________________________ > LLVM Developers mailing listLLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.eduhttp://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > > > > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20130808/a985d657/attachment.html>
I believe we were under the impression that InstCombine, as a canonicalized/optimizer, should not increase code size but only reduce it. Minor aside, but you don't need all of fast-math for the IR, just the "arcp" flag, which allows for replacement of division with reciprocal-multiply. On Aug 8, 2013, at 10:21 AM, Shuxin Yang <shuxin.llvm at gmail.com> wrote:> I remember why I didn't implement this rule in Instcombine. It add one instruction. So, > this xform should be driven by a redundancy eliminator if you care code size. > > On 8/8/13 10:13 AM, Shuxin Yang wrote: >> I did few transformation in Instruction *InstCombiner::visitFDiv() in an attempt to remove some divs. >> I may miss this case. If you need to implement this rule, it is better done in Instcombine than in DAG combine. >> Doing such xform early expose the redundancy of 1/y, which have positive impact to neighboring code, >> while DAG combine is bit blind. >> >> You should be very careful, reciprocal is very very very imprecise transformation. Sometimes you will see big different >> with and without this xform. >> >> On 8/8/13 9:25 AM, Chad Rosier wrote: >>> I would like to transform X/Y -> X*1/Y. Specifically, I would like to convert: >>> >>> define void @t1a(double %a, double %b, double %d) { >>> entry: >>> %div = fdiv fast double %a, %d >>> %div1 = fdiv fast double %b, %d >>> %call = tail call i32 @foo(double %div, double %div1) >>> ret void >>> } >>> >>> to: >>> >>> define void @t1b(double %a, double %b, double %d) { >>> entry: >>> %div = fdiv fast double 1.000000e+00, %d >>> %mul = fmul fast double %div, %a >>> %mul1 = fmul fast double %div, %b >>> %call = tail call i32 @foo(double %mul, double %mul1) >>> ret void >>> } >>> >>> Is such a transformation best done as a (target-specific) DAG combine? >>> >>> A similar instcombine already exists for the X/C->X*1/C case (see the CvtFDivConstToReciprocal function in InstCombineMlDivRem.cpp), but I don't believe the above can be done as an instcombine as it creates a new instruction (in addition to replacing the original). Also, I only want to perform the transformation if there are multiple uses of 1/Y (like in my test case). Otherwise, the transformation replaces a fdiv with a fdiv+fmul pair, which I doubt would be profitable. >>> >>> FWIW, I'm also pretty sure this combine requires -fast-math. >>> >>> Can someone point me in the right direction? >>> >>> Thanks, >>> Chad >>> >>> >>> _______________________________________________ >>> LLVM Developers mailing list >>> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu >>> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >> > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20130808/d8e4ece0/attachment.html>
On 8/8/13 10:40 AM, Michael Ilseman wrote:> I believe we were under the impression that InstCombine, as a > canonicalized/optimizer, should not increase code size but only reduce > it. > > Minor aside, but you don't need all of fast-math for the IR, just the > "arcp" flag, which allows for replacement of division with > reciprocal-multiply.I vaguely remember we are not quite happy with the arcp relax rank because the it is not very relax, while the the reciprocal is very imprecise. I guess we are better off using unsafe-math to guard such xform?> > On Aug 8, 2013, at 10:21 AM, Shuxin Yang <shuxin.llvm at gmail.com > <mailto:shuxin.llvm at gmail.com>> wrote: > >> I remember why I didn't implement this rule in Instcombine. It add >> one instruction. So, >> this xform should be driven by a redundancy eliminator if you care >> code size. >> >> On 8/8/13 10:13 AM, Shuxin Yang wrote: >>> I did few transformation in Instruction *InstCombiner::visitFDiv() >>> in an attempt to remove some divs. >>> I may miss this case. If you need to implement this rule, it is >>> better done in Instcombine than in DAG combine. >>> Doing such xform early expose the redundancy of 1/y, which have >>> positive impact to neighboring code, >>> while DAG combine is bit blind. >>> >>> You should be very careful, reciprocal is very very very imprecise >>> transformation. Sometimes you will see big different >>> with and without this xform. >>> >>> On 8/8/13 9:25 AM, Chad Rosier wrote: >>>> I would like to transform X/Y -> X*1/Y. Specifically, I would like >>>> to convert: >>>> >>>> define void @t1a(double %a, double %b, double %d) { >>>> entry: >>>> %div = fdiv fast double %a, %d >>>> %div1 = fdiv fast double %b, %d >>>> %call = tail call i32 @foo(double %div, double %div1) >>>> ret void >>>> } >>>> >>>> to: >>>> >>>> define void @t1b(double %a, double %b, double %d) { >>>> entry: >>>> %div = fdiv fast double 1.000000e+00, %d >>>> %mul = fmul fast double %div, %a >>>> %mul1 = fmul fast double %div, %b >>>> %call = tail call i32 @foo(double %mul, double %mul1) >>>> ret void >>>> } >>>> >>>> Is such a transformation best done as a (target-specific) DAG combine? >>>> >>>> A similar instcombine already exists for the X/C->X*1/C case (see >>>> the CvtFDivConstToReciprocal function in InstCombineMlDivRem.cpp), >>>> but I don't believe the above can be done as an instcombine as it >>>> creates a new instruction (in addition to replacing the original). >>>> Also, I only want to perform the transformation if there are >>>> multiple uses of 1/Y (like in my test case). Otherwise, the >>>> transformation replaces a fdiv with a fdiv+fmul pair, which I doubt >>>> would be profitable. >>>> >>>> FWIW, I'm also pretty sure this combine requires -fast-math. >>>> >>>> Can someone point me in the right direction? >>>> >>>> Thanks, >>>> Chad >>>> >>>> >>>> _______________________________________________ >>>> LLVM Developers mailing list >>>> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu >>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >>> >> >> _______________________________________________ >> LLVM Developers mailing list >> LLVMdev at cs.uiuc.edu <mailto:LLVMdev at cs.uiuc.edu> http://llvm.cs.uiuc.edu >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20130808/fd3503cf/attachment.html>