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

[Philip Reames via llvm-commits]

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/20180308/1c100689/attachment.html>