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

[Chad Rosier]

Hi Kevin,
I've summarized the issue here:
http://llvm.org/bugs/show_bug.cgi?id=21600

I'll try and commit a fix shortly.  Hopefully, the net effect will be a
fix for mcf and improvements in other code.

 Chad

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