raghesh via llvm-dev
2019-Dec-31 00:06 UTC
[llvm-dev] Any significance for m_OneUse in (X / Y) / Z => X / (Y * Z) ??
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. ------------------------------ -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20191231/2cdbce17/attachment.html>
Craig Topper via llvm-dev
2019-Dec-31 00:11 UTC
[llvm-dev] Any significance for m_OneUse in (X / Y) / Z => X / (Y * Z) ??
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 >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20191230/7b126523/attachment.html>
raghesh via llvm-dev
2019-Dec-31 06:09 UTC
[llvm-dev] Any significance for m_OneUse in (X / Y) / Z => X / (Y * Z) ??
Thanks a lot Craig. That is convincing. Regards, ------------------------------ Raghesh Aloor AMD India Pvt. Ltd. Bengaluru. ------------------------------ On Tue, Dec 31, 2019 at 5:41 AM Craig Topper <craig.topper at gmail.com> 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 >> >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20191231/62d40954/attachment.html>
Sanjay Patel via llvm-dev
2020-Jan-03 14:32 UTC
[llvm-dev] Any significance for m_OneUse in (X / Y) / Z => X / (Y * Z) ??
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/20200103/6a924184/attachment.html>