[llvm-dev] Any significance for m_OneUse in (X / Y) / Z => X / (Y * Z) ??

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

[llvm-dev] Any significance for m_OneUse in (X / Y) / Z => X / (Y * Z) ??

Adrian Prantl via llvm-dev
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
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
Reply | Threaded
Open this post in threaded view
|

Re: [llvm-dev] Any significance for m_OneUse in (X / Y) / Z => X / (Y * Z) ??

Adrian Prantl via llvm-dev
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 <[hidden email]> 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
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

_______________________________________________
LLVM Developers mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
Reply | Threaded
Open this post in threaded view
|

Re: [llvm-dev] Any significance for m_OneUse in (X / Y) / Z => X / (Y * Z) ??

Adrian Prantl via llvm-dev
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 <[hidden email]> 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 <[hidden email]> 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
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

_______________________________________________
LLVM Developers mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
Reply | Threaded
Open this post in threaded view
|

Re: [llvm-dev] Any significance for m_OneUse in (X / Y) / Z => X / (Y * Z) ??

Adrian Prantl via llvm-dev
In reply to this post by Adrian Prantl via llvm-dev
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 <[hidden email]> 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 <[hidden email]> 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
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
_______________________________________________
LLVM Developers mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

_______________________________________________
LLVM Developers mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
Reply | Threaded
Open this post in threaded view
|

Re: [llvm-dev] Any significance for m_OneUse in (X / Y) / Z => X / (Y * Z) ??

Adrian Prantl via llvm-dev
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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
_______________________________________________
LLVM Developers mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

_______________________________________________
LLVM Developers mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
Reply | Threaded
Open this post in threaded view
|

Re: [llvm-dev] Any significance for m_OneUse in (X / Y) / Z => X / (Y * Z) ??

Adrian Prantl via llvm-dev
Is your case the case mentioned in the subject or a different case?

~Craig


On Sun, Jan 5, 2020 at 8:11 PM raghesh <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
_______________________________________________
LLVM Developers mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

_______________________________________________
LLVM Developers mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
Reply | Threaded
Open this post in threaded view
|

Re: [llvm-dev] Any significance for m_OneUse in (X / Y) / Z => X / (Y * Z) ??

Adrian Prantl via llvm-dev
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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
_______________________________________________
LLVM Developers mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

_______________________________________________
LLVM Developers mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
Reply | Threaded
Open this post in threaded view
|

Re: [llvm-dev] Any significance for m_OneUse in (X / Y) / Z => X / (Y * Z) ??

Adrian Prantl via llvm-dev
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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
_______________________________________________
LLVM Developers mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

_______________________________________________
LLVM Developers mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
Reply | Threaded
Open this post in threaded view
|

Re: [llvm-dev] Any significance for m_OneUse in (X / Y) / Z => X / (Y * Z) ??

Adrian Prantl via llvm-dev
In reply to this post by Adrian Prantl via llvm-dev
If the transform is a performance optimization despite potentially increasing the number of instructions, that should go in the backend. Most likely that will be in DAGCombiner or a target-specific lowering file.

But as noted in the later reply, if this question is really about a reciprocal special-case, that becomes an extension of what we already have in instcombine.

To be clear - we're talking about FP math, and all of these transforms require some form of fast-math-flags to be valid (no matter where they are implemented).


On Sun, Jan 5, 2020 at 11:11 PM raghesh <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
_______________________________________________
LLVM Developers mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

_______________________________________________
LLVM Developers mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
Reply | Threaded
Open this post in threaded view
|

Re: [llvm-dev] Any significance for m_OneUse in (X / Y) / Z => X / (Y * Z) ??

Adrian Prantl via llvm-dev
Thank You.

Yes. It is a reciprocal special-case and expected to be invoked only in -fast-math.

Regards,
------------------------------
Raghesh Aloor
AMD India Pvt. Ltd.
Bengaluru.
------------------------------


On Mon, Jan 6, 2020 at 6:28 PM Sanjay Patel <[hidden email]> wrote:
If the transform is a performance optimization despite potentially increasing the number of instructions, that should go in the backend. Most likely that will be in DAGCombiner or a target-specific lowering file.

But as noted in the later reply, if this question is really about a reciprocal special-case, that becomes an extension of what we already have in instcombine.

To be clear - we're talking about FP math, and all of these transforms require some form of fast-math-flags to be valid (no matter where they are implemented).


On Sun, Jan 5, 2020 at 11:11 PM raghesh <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
_______________________________________________
LLVM Developers mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

_______________________________________________
LLVM Developers mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev