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

[Chad Rosier]

Hi Kevin,
I'll take a look today.  If you find any other issues, please let me know.

 Chad

> 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
>