Craig Topper via llvm-dev
2020-Jan-06 04:43 UTC
[llvm-dev] Any significance for m_OneUse in (X / Y) / Z => X / (Y * Z) ??
Is your case the case mentioned in the subject or a different case? ~Craig On Sun, Jan 5, 2020 at 8:11 PM raghesh <raghesh.a at gmail.com> wrote:> Thanks Sanjay for your comments. > > So, if we want to add a transformation avoiding the one-use check, which > one is the ideal pass? Shall we do it in -aggressive-instcombine? I came > to know that if the pattern search complexity is O(n) [1] we should go for > aggressive-instcombine. If it is O(1) we must do that in -instcombine. > However, in my case, the complexity is still O(1) and want to avoid the > one-use check. > > [1] n is the number of instructions in the function. > > Regards, > ------------------------------ > Raghesh Aloor > AMD India Pvt. Ltd. > Bengaluru. > ------------------------------ > > > On Fri, Jan 3, 2020 at 8:02 PM Sanjay Patel <spatel at rotateright.com> > wrote: > >> A couple more general comments: >> 1. There shouldn't be any correctness issues removing one-use checks (the >> transform should be safe independently of use-counts). >> 2. Ideally, you can remove a m_OneUse() from the code and run 'make >> check' or similar, and you will see a regression test failure because we >> have a 'negative' test to cover that pattern. That should make it clear >> that the one-use check is there intentionally and what the effect of >> removing it is. We've gotten better about including those kinds of >> regression tests over time, but I don't know what percentage of all >> instcombine transforms actually have that test coverage. >> >> On Mon, Dec 30, 2019 at 7:12 PM Craig Topper via llvm-dev < >> llvm-dev at lists.llvm.org> wrote: >> >>> As a general rule, InstCombine tries not increase the total number of >>> instructions. If X/Y has another use other than this one, then it still >>> ends up being calculated. Without the one use check you'd trade 2 fdivs, >>> for 1 mul (Y * Z), and 2 fdivs ((X*Y)/Z) and the original (X / Y). >>> >>> ~Craig >>> >>> >>> On Mon, Dec 30, 2019 at 4:07 PM raghesh via llvm-dev < >>> llvm-dev at lists.llvm.org> wrote: >>> >>>> Dear All, >>>> >>>> The InstCombine pass performs the following transformation. >>>> >>>> Z / (X / Y) => (Y * Z) / X >>>> >>>> This is performed only when operand Op1 ( (X/Y) in this case) has only >>>> one use in future. The code snippet is shown below. >>>> >>>> if (match(Op1, m_OneUse(m_FDiv(m_Value(X), m_Value(Y)))) && >>>> (!isa<Constant>(Y) || !isa<Constant>(Op0))) { >>>> // Z / (X / Y) => (Y * Z) / X >>>> Value *YZ = Builder.CreateFMulFMF(Y, Op0, &I); >>>> return BinaryOperator::CreateFDivFMF(YZ, X, &I); >>>> } >>>> >>>> It would be great if someone explains if there is any issue >>>> (correctness/performance-wise) if we avoid the m_OueUse check. What if >>>> we perform the transformation even if there are multiple uses? >>>> >>>> There are similar transformations which perform the m_OueUse check. We >>>> may avoid those too if there is no particular reason for the check. >>>> >>>> Regards, >>>> ------------------------------ >>>> Raghesh Aloor >>>> AMD India Pvt. Ltd. >>>> Bengaluru. >>>> ------------------------------ >>>> _______________________________________________ >>>> LLVM Developers mailing list >>>> llvm-dev at lists.llvm.org >>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >>>> >>> _______________________________________________ >>> LLVM Developers mailing list >>> llvm-dev at lists.llvm.org >>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >>> >>-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200105/25a351d0/attachment.html>
raghesh via llvm-dev
2020-Jan-06 06:29 UTC
[llvm-dev] Any significance for m_OneUse in (X / Y) / Z => X / (Y * Z) ??
Here is my case. Z / (1.0 / Y) => (Y * Z) This is similar to Z / (X / Y) => (Y * Z) / X. Currently, the former one is prohibitted because of the one-use check. In the latter, as you explained earlier, the number of instructions are increased from 2 to 3. However, in the former case (where X = 1.0), the number of instructions remain the same as the division by 1.0 is avoided. Additionally, instead of a division, now we have a multiplication. This potentially may reduce the number of instruction cycles. Regards, ------------------------------ Raghesh Aloor AMD India Pvt. Ltd. Bengaluru. ------------------------------ On Mon, Jan 6, 2020 at 10:14 AM Craig Topper <craig.topper at gmail.com> wrote:> Is your case the case mentioned in the subject or a different case? > > ~Craig > > > On Sun, Jan 5, 2020 at 8:11 PM raghesh <raghesh.a at gmail.com> wrote: > >> Thanks Sanjay for your comments. >> >> So, if we want to add a transformation avoiding the one-use check, which >> one is the ideal pass? Shall we do it in -aggressive-instcombine? I came >> to know that if the pattern search complexity is O(n) [1] we should go for >> aggressive-instcombine. If it is O(1) we must do that in -instcombine. >> However, in my case, the complexity is still O(1) and want to avoid the >> one-use check. >> >> [1] n is the number of instructions in the function. >> >> Regards, >> ------------------------------ >> Raghesh Aloor >> AMD India Pvt. Ltd. >> Bengaluru. >> ------------------------------ >> >> >> On Fri, Jan 3, 2020 at 8:02 PM Sanjay Patel <spatel at rotateright.com> >> wrote: >> >>> A couple more general comments: >>> 1. There shouldn't be any correctness issues removing one-use checks >>> (the transform should be safe independently of use-counts). >>> 2. Ideally, you can remove a m_OneUse() from the code and run 'make >>> check' or similar, and you will see a regression test failure because we >>> have a 'negative' test to cover that pattern. That should make it clear >>> that the one-use check is there intentionally and what the effect of >>> removing it is. We've gotten better about including those kinds of >>> regression tests over time, but I don't know what percentage of all >>> instcombine transforms actually have that test coverage. >>> >>> On Mon, Dec 30, 2019 at 7:12 PM Craig Topper via llvm-dev < >>> llvm-dev at lists.llvm.org> wrote: >>> >>>> As a general rule, InstCombine tries not increase the total number of >>>> instructions. If X/Y has another use other than this one, then it still >>>> ends up being calculated. Without the one use check you'd trade 2 fdivs, >>>> for 1 mul (Y * Z), and 2 fdivs ((X*Y)/Z) and the original (X / Y). >>>> >>>> ~Craig >>>> >>>> >>>> On Mon, Dec 30, 2019 at 4:07 PM raghesh via llvm-dev < >>>> llvm-dev at lists.llvm.org> wrote: >>>> >>>>> Dear All, >>>>> >>>>> The InstCombine pass performs the following transformation. >>>>> >>>>> Z / (X / Y) => (Y * Z) / X >>>>> >>>>> This is performed only when operand Op1 ( (X/Y) in this case) has only >>>>> one use in future. The code snippet is shown below. >>>>> >>>>> if (match(Op1, m_OneUse(m_FDiv(m_Value(X), m_Value(Y)))) && >>>>> (!isa<Constant>(Y) || !isa<Constant>(Op0))) { >>>>> // Z / (X / Y) => (Y * Z) / X >>>>> Value *YZ = Builder.CreateFMulFMF(Y, Op0, &I); >>>>> return BinaryOperator::CreateFDivFMF(YZ, X, &I); >>>>> } >>>>> >>>>> It would be great if someone explains if there is any issue >>>>> (correctness/performance-wise) if we avoid the m_OueUse check. What >>>>> if we perform the transformation even if there are multiple uses? >>>>> >>>>> There are similar transformations which perform the m_OueUse check. >>>>> We may avoid those too if there is no particular reason for the check. >>>>> >>>>> Regards, >>>>> ------------------------------ >>>>> Raghesh Aloor >>>>> AMD India Pvt. Ltd. >>>>> Bengaluru. >>>>> ------------------------------ >>>>> _______________________________________________ >>>>> LLVM Developers mailing list >>>>> llvm-dev at lists.llvm.org >>>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >>>>> >>>> _______________________________________________ >>>> LLVM Developers mailing list >>>> llvm-dev at lists.llvm.org >>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >>>> >>>-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200106/85fddc78/attachment.html>
Craig Topper via llvm-dev
2020-Jan-06 06:38 UTC
[llvm-dev] Any significance for m_OneUse in (X / Y) / Z => X / (Y * Z) ??
You can match that in InstCombine without a one use check if you explicitly match the 1.0. I think we generally understand that multiply is better than divide. ~Craig On Sun, Jan 5, 2020 at 10:29 PM raghesh <raghesh.a at gmail.com> wrote:> Here is my case. > > Z / (1.0 / Y) => (Y * Z) > > This is similar to Z / (X / Y) => (Y * Z) / X. Currently, the former one > is prohibitted because of the one-use check. In the latter, as you > explained earlier, the number of instructions are increased from 2 to 3. > However, in the former case (where X = 1.0), the number of instructions > remain the same as the division by 1.0 is avoided. Additionally, instead of > a division, now we have a multiplication. This potentially may reduce the > number of instruction cycles. > > Regards, > ------------------------------ > Raghesh Aloor > AMD India Pvt. Ltd. > Bengaluru. > ------------------------------ > > > On Mon, Jan 6, 2020 at 10:14 AM Craig Topper <craig.topper at gmail.com> > wrote: > >> Is your case the case mentioned in the subject or a different case? >> >> ~Craig >> >> >> On Sun, Jan 5, 2020 at 8:11 PM raghesh <raghesh.a at gmail.com> wrote: >> >>> Thanks Sanjay for your comments. >>> >>> So, if we want to add a transformation avoiding the one-use check, which >>> one is the ideal pass? Shall we do it in -aggressive-instcombine? I >>> came to know that if the pattern search complexity is O(n) [1] we should go >>> for aggressive-instcombine. If it is O(1) we must do that in - >>> instcombine. However, in my case, the complexity is still O(1) and want >>> to avoid the one-use check. >>> >>> [1] n is the number of instructions in the function. >>> >>> Regards, >>> ------------------------------ >>> Raghesh Aloor >>> AMD India Pvt. Ltd. >>> Bengaluru. >>> ------------------------------ >>> >>> >>> On Fri, Jan 3, 2020 at 8:02 PM Sanjay Patel <spatel at rotateright.com> >>> wrote: >>> >>>> A couple more general comments: >>>> 1. There shouldn't be any correctness issues removing one-use checks >>>> (the transform should be safe independently of use-counts). >>>> 2. Ideally, you can remove a m_OneUse() from the code and run 'make >>>> check' or similar, and you will see a regression test failure because we >>>> have a 'negative' test to cover that pattern. That should make it clear >>>> that the one-use check is there intentionally and what the effect of >>>> removing it is. We've gotten better about including those kinds of >>>> regression tests over time, but I don't know what percentage of all >>>> instcombine transforms actually have that test coverage. >>>> >>>> On Mon, Dec 30, 2019 at 7:12 PM Craig Topper via llvm-dev < >>>> llvm-dev at lists.llvm.org> wrote: >>>> >>>>> As a general rule, InstCombine tries not increase the total number of >>>>> instructions. If X/Y has another use other than this one, then it still >>>>> ends up being calculated. Without the one use check you'd trade 2 fdivs, >>>>> for 1 mul (Y * Z), and 2 fdivs ((X*Y)/Z) and the original (X / Y). >>>>> >>>>> ~Craig >>>>> >>>>> >>>>> On Mon, Dec 30, 2019 at 4:07 PM raghesh via llvm-dev < >>>>> llvm-dev at lists.llvm.org> wrote: >>>>> >>>>>> Dear All, >>>>>> >>>>>> The InstCombine pass performs the following transformation. >>>>>> >>>>>> Z / (X / Y) => (Y * Z) / X >>>>>> >>>>>> This is performed only when operand Op1 ( (X/Y) in this case) has >>>>>> only one use in future. The code snippet is shown below. >>>>>> >>>>>> if (match(Op1, m_OneUse(m_FDiv(m_Value(X), m_Value(Y)))) && >>>>>> (!isa<Constant>(Y) || !isa<Constant>(Op0))) { >>>>>> // Z / (X / Y) => (Y * Z) / X >>>>>> Value *YZ = Builder.CreateFMulFMF(Y, Op0, &I); >>>>>> return BinaryOperator::CreateFDivFMF(YZ, X, &I); >>>>>> } >>>>>> >>>>>> It would be great if someone explains if there is any issue >>>>>> (correctness/performance-wise) if we avoid the m_OueUse check. What >>>>>> if we perform the transformation even if there are multiple uses? >>>>>> >>>>>> There are similar transformations which perform the m_OueUse check. >>>>>> We may avoid those too if there is no particular reason for the check. >>>>>> >>>>>> Regards, >>>>>> ------------------------------ >>>>>> Raghesh Aloor >>>>>> AMD India Pvt. Ltd. >>>>>> Bengaluru. >>>>>> ------------------------------ >>>>>> _______________________________________________ >>>>>> LLVM Developers mailing list >>>>>> llvm-dev at lists.llvm.org >>>>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >>>>>> >>>>> _______________________________________________ >>>>> LLVM Developers mailing list >>>>> llvm-dev at lists.llvm.org >>>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >>>>> >>>>-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200105/1b9471de/attachment.html>