[llvm] r326861 - Add early exit on reassociation of 0 expression.

[Evgeny Stupachenko via llvm-commits]

 >In the scenario you gave, mul is a binary operator, 0 is not.  Calling
tryReassociateBinary on the operand would be suspicious.
Ok. "tryReassociateBinary" is called on "%1 = mul nsw i8 0, %0 ", not "0".
But SCEV of "%1 = mul nsw i8 0, %0 " is "0". What is wrong/inconsistent
here?
There could be non-zero (non-constant) value with "0" SCEV. It could be
(and most likely will be) optimized, but not in nary reassociation pass.

>%1 = mul nsw i8 42, %0
This has non-Zero SCEV (and non-constant). If it is the only given
instruction, SCEV should be 42 * %0.

Please read comments in http://reviews.llvm.org/D41467 on how Nary
Reassociation fall into infinite loop.

On Thu, Mar 8, 2018 at 4:28 PM, Philip Reames <listmail at philipreames.com>
wrote:

> In the scenario you gave, mul is a binary operator, 0 is not.  Calling
> tryReassociateBinary on the operand would be suspicious.
> %1 = mul nsw i8 42, %0
>
> (I'd also not expect '42' to be a binary operator in the above.  Having i8
> 42 be an argument to tryReassociateBinaryOp would also seem wrong.)
>
>
> On 03/08/2018 02:43 PM, Evgeny Stupachenko wrote:
>
> Well, is
>
>  %1 = mul nsw i8 0, %0
>
> binary operator?
> It has Zero SCEV.
>
>
> On Thu, Mar 8, 2018 at 1:57 PM, Philip Reames <listmail at philipreames.com>
> wrote:
>
>> Huh?  Zero is not a binary operator.  So how'd it get here?  This patch
>> looks wrong or at least incomplete.
>>
>> Philip
>>
>>
>>
>> On 03/06/2018 06:17 PM, Evgeny Stupachenko via llvm-commits wrote:
>>
>>> Author: evstupac
>>> Date: Tue Mar  6 18:17:08 2018
>>> New Revision: 326861
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=326861&view=rev
>>> Log:
>>> Add early exit on reassociation of 0 expression.
>>>
>>> Summary:
>>>
>>> Before the patch a try to reassociate ((v * 16) * 0) * 1 fall into
>>> infinite loop
>>>
>>> Reviewers: pankajchawla
>>>
>>> Differential Revision: http://reviews.llvm.org/D41467
>>>
>>> From: Evgeny Stupachenko <evstupac at gmail.com>
>>>                           <evgeny.v.stupachenko at intel.com>
>>>
>>> Added:
>>>      llvm/trunk/test/Transforms/NaryReassociate/pr35710.ll
>>> Modified:
>>>      llvm/trunk/lib/Transforms/Scalar/NaryReassociate.cpp
>>>
>>> Modified: llvm/trunk/lib/Transforms/Scalar/NaryReassociate.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transform
>>> s/Scalar/NaryReassociate.cpp?rev=326861&r1=326860&r2=326861&view=diff
>>> ============================================================
>>> ==================
>>> --- llvm/trunk/lib/Transforms/Scalar/NaryReassociate.cpp (original)
>>> +++ llvm/trunk/lib/Transforms/Scalar/NaryReassociate.cpp Tue Mar  6
>>> 18:17:08 2018
>>> @@ -429,6 +429,9 @@ NaryReassociatePass::tryReassociateGEPAt
>>>     Instruction *NaryReassociatePass::tryReassociateBinaryOp(BinaryOperator
>>> *I) {
>>>     Value *LHS = I->getOperand(0), *RHS = I->getOperand(1);
>>> +  // There is no need to reassociate 0.
>>> +  if (SE->getSCEV(I)->isZero())
>>> +    return nullptr;
>>>     if (auto *NewI = tryReassociateBinaryOp(LHS, RHS, I))
>>>       return NewI;
>>>     if (auto *NewI = tryReassociateBinaryOp(RHS, LHS, I))
>>>
>>> Added: llvm/trunk/test/Transforms/NaryReassociate/pr35710.ll
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transfor
>>> ms/NaryReassociate/pr35710.ll?rev=326861&view=auto
>>> ============================================================
>>> ==================
>>> --- llvm/trunk/test/Transforms/NaryReassociate/pr35710.ll (added)
>>> +++ llvm/trunk/test/Transforms/NaryReassociate/pr35710.ll Tue Mar  6
>>> 18:17:08 2018
>>> @@ -0,0 +1,19 @@
>>> +; NOTE: Assertions have been autogenerated by
>>> utils/update_test_checks.py
>>> +; RUN: opt < %s -nary-reassociate -S | FileCheck %s
>>> +
>>> +; The test check that compilation does not fall into infinite loop.
>>> +
>>> +define i8 @foo(i8 %v) local_unnamed_addr #0 {
>>> +; CHECK-LABEL: @foo(
>>> +; CHECK-NEXT:  region.0:
>>> +; CHECK-NEXT:    [[TMP0:%.*]] = mul nsw i8 16, [[V:%.*]]
>>> +; CHECK-NEXT:    [[TMP1:%.*]] = mul nsw i8 0, [[TMP0]]
>>> +; CHECK-NEXT:    [[TMP2:%.*]] = mul nsw i8 1, [[TMP1]]
>>> +; CHECK-NEXT:    ret i8 [[TMP2]]
>>> +;
>>> +region.0:
>>> +  %0 = mul nsw i8 16, %v
>>> +  %1 = mul nsw i8 0, %0
>>> +  %2 = mul nsw i8 1, %1
>>> +  ret i8 %2
>>> +}
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>
>>
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180308/32cacda7/attachment.html>