llvm dev - Jan 2020 - 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>

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>

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>

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>