[llvm] r326861 - Add early exit on reassociation of 0 expression.
Philip Reames via llvm-commits
llvm-commits at lists.llvm.org
Fri Mar 9 19:23:23 PST 2018
I see your point now, and you are correct. I retract my concern; thanks
for explaining.
Philip
On 03/08/2018 05:03 PM, Evgeny Stupachenko wrote:
> >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 <mailto: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 <mailto: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
>> <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
>> <mailto:evstupac at gmail.com>>
>> <evgeny.v.stupachenko at intel.com
>> <mailto: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/Transforms/Scalar/NaryReassociate.cpp?rev=326861&r1=326860&r2=326861&view=diff
>> <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/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/Transforms/NaryReassociate/pr35710.ll?rev=326861&view=auto
>> <http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/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
>> <mailto:llvm-commits at lists.llvm.org>
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>> <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/20180309/afa0109d/attachment.html>
More information about the llvm-commits
mailing list