[llvm] r222008 - [Reassociate] Canonicalize the operands of all binary operators.

[Kevin Qin]

Hi Chad,

On cortex-a57, we found a 3.8% performance regression on 429.mcf in
spec2006 due to this commit, would you have a look of this?

The branch order changed by this commit.

Before:
  tbz x8, #63, 4028e0 <primal_bea_mpp+0x250>
  cmp w12, #0x1
  b.eq 4028ec <primal_bea_mpp+0x25c>


After:
  cmp w12, #0x1
  b.ne 40290c <primal_bea_mpp+0x250>
  tbnz x8, #63, 402918 <primal_bea_mpp+0x25c>

This change happens in function primal_bea_mpp and got performance worse.

I didn't look into detail, that's all what I know from a simple
investigation. Hopefully this can give you a clue to fix it.

Thanks,
Kevin

2014-11-14 17:09 GMT+00:00 Chad Rosier <mcrosier at codeaurora.org>:

> Author: mcrosier
> Date: Fri Nov 14 11:09:19 2014
> New Revision: 222008
>
> URL: http://llvm.org/viewvc/llvm-project?rev=222008&view=rev
> Log:
> [Reassociate] Canonicalize the operands of all binary operators.
>
> Added:
>     llvm/trunk/test/Transforms/Reassociate/commute.ll
> Modified:
>     llvm/trunk/lib/Transforms/Scalar/Reassociate.cpp
>     llvm/trunk/test/Transforms/Reassociate/multistep.ll
>
> Modified: llvm/trunk/lib/Transforms/Scalar/Reassociate.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/Reassociate.cpp?rev=222008&r1=222007&r2=222008&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Scalar/Reassociate.cpp (original)
> +++ llvm/trunk/lib/Transforms/Scalar/Reassociate.cpp Fri Nov 14 11:09:19
> 2014
> @@ -2078,19 +2078,19 @@ void Reassociate::OptimizeInst(Instructi
>    if (Instruction *Res = canonicalizeNegConstExpr(I))
>      I = Res;
>
> -  // Commute floating point binary operators, to canonicalize the order
> of their
> -  // operands.  This can potentially expose more CSE opportunities, and
> makes
> -  // writing other transformations simpler.
> -  if (I->getType()->isFloatingPointTy() || I->getType()->isVectorTy()) {
> +  // Commute binary operators, to canonicalize the order of their
> operands.
> +  // This can potentially expose more CSE opportunities, and makes
> writing other
> +  // transformations simpler.
> +  if (I->isCommutative())
> +    canonicalizeOperands(I);
>
> -    if (I->isCommutative())
> -      canonicalizeOperands(I);
> +  // Don't optimize vector instructions.
> +  if (I->getType()->isVectorTy())
> +    return;
>
> -    // Don't try to optimize vector instructions or anything that doesn't
> have
> -    // unsafe algebra.
> -    if (I->getType()->isVectorTy() || !I->hasUnsafeAlgebra())
> -      return;
> -  }
> +  // Don't optimize floating point instructions that don't have unsafe
> algebra.
> +  if (I->getType()->isFloatingPointTy() && !I->hasUnsafeAlgebra())
> +    return;
>
>    // Do not reassociate boolean (i1) expressions.  We want to preserve the
>    // original order of evaluation for short-circuited comparisons that
>
> Added: llvm/trunk/test/Transforms/Reassociate/commute.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/Reassociate/commute.ll?rev=222008&view=auto
>
> ==============================================================================
> --- llvm/trunk/test/Transforms/Reassociate/commute.ll (added)
> +++ llvm/trunk/test/Transforms/Reassociate/commute.ll Fri Nov 14 11:09:19
> 2014
> @@ -0,0 +1,19 @@
> +; RUN: opt -reassociate -S < %s | FileCheck %s
> +
> +declare void @use(i32)
> +
> +define void @test1(i32 %x, i32 %y) {
> +; CHECK-LABEL: test1
> +; CHECK: mul i32 %y, %x
> +; CHECK: mul i32 %y, %x
> +; CHECK: sub i32 %1, %2
> +; CHECK: call void @use(i32 %{{.*}})
> +; CHECK: call void @use(i32 %{{.*}})
> +
> +  %1 = mul i32 %x, %y
> +  %2 = mul i32 %y, %x
> +  %3 = sub i32 %1, %2
> +  call void @use(i32 %1)
> +  call void @use(i32 %3)
> +  ret void
> +}
>
> Modified: llvm/trunk/test/Transforms/Reassociate/multistep.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/Reassociate/multistep.ll?rev=222008&r1=222007&r2=222008&view=diff
>
> ==============================================================================
> --- llvm/trunk/test/Transforms/Reassociate/multistep.ll (original)
> +++ llvm/trunk/test/Transforms/Reassociate/multistep.ll Fri Nov 14
> 11:09:19 2014
> @@ -9,7 +9,7 @@ define i64 @multistep1(i64 %a, i64 %b, i
>    %t3 = mul i64 %a, %t2 ; a*(a*c)
>    %t4 = add i64 %t1, %t3
>  ; CHECK-NEXT: add i64 %c, %b
> -; CHECK-NEXT: mul i64 %tmp{{.*}}, %a
> +; CHECK-NEXT: mul i64 %a, %tmp{{.*}}
>  ; CHECK-NEXT: mul i64 %tmp{{.*}}, %a
>  ; CHECK-NEXT: ret
>    ret i64 %t4
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>



-- 
Best Regards,

Kevin Qin
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141118/7ec33f4f/attachment.html>