[llvm] r247705 - [NaryReassociate] Add support for Mul instructions
Marcello Maggioni via llvm-commits
llvm-commits at lists.llvm.org
Tue Sep 15 11:13:53 PDT 2015
Isn’t this test in “nary-add.ll” doing exactly the same ?
define void @left_reassociate(i32 %a, i32 %b, i32 %c) {
; CHECK-LABEL: @left_reassociate(
%1 = add i32 %a, %c
; CHECK: [[BASE:%[a-zA-Z0-9]+]] = add i32 %a, %c
call void @foo(i32 %1)
%2 = add i32 %b, %c
%3 = add i32 %a, %2
; CHECK: add i32 [[BASE]], %b
call void @foo(i32 %3)
ret void
}
Or maybe I missed the point of the issue you were reporting?
Marcello
> On 15 Sep 2015, at 11:07, Mehdi Amini <mehdi.amini at apple.com> wrote:
>
>
>> On Sep 15, 2015, at 11:03 AM, Marcello Maggioni <mmaggioni at apple.com <mailto:mmaggioni at apple.com>> wrote:
>>
>> I think Volkan just used the same template of the tests in the sibling “nary-add.ll” test file.
>>
>> If you think the test is not adequate probably all of them should be fixed to a more consistent format?
>
> There is only one other test in this directory (as far as I can see), and it does not suffer from what I’m reporting.
>
> —
> Mehdi
>
>
>> Volkan, do you have a comment on that?
>>
>> Marcello
>>> On 15 Sep 2015, at 10:29, Mehdi Amini via llvm-commits <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> wrote:
>>>
>>>>
>>>> On Sep 15, 2015, at 10:22 AM, Marcello Maggioni via llvm-commits <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> wrote:
>>>>
>>>> Author: mggm
>>>> Date: Tue Sep 15 12:22:52 2015
>>>> New Revision: 247705
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=247705&view=rev <http://llvm.org/viewvc/llvm-project?rev=247705&view=rev>
>>>> Log:
>>>> [NaryReassociate] Add support for Mul instructions
>>>>
>>>> This patch extends the current pass by handling
>>>> Mul instructions as well.
>>>>
>>>> Patch by: Volkan Keles (vkeles at apple.com <mailto:vkeles at apple.com>)
>>>>
>>>> Added:
>>>> llvm/trunk/test/Transforms/NaryReassociate/nary-mul.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=247705&r1=247704&r2=247705&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/NaryReassociate.cpp?rev=247705&r1=247704&r2=247705&view=diff>
>>>> ==============================================================================
>>>> --- llvm/trunk/lib/Transforms/Scalar/NaryReassociate.cpp (original)
>>>> +++ llvm/trunk/lib/Transforms/Scalar/NaryReassociate.cpp Tue Sep 15 12:22:52 2015
>>>> @@ -71,8 +71,8 @@
>>>> //
>>>> // Limitations and TODO items:
>>>> //
>>>> -// 1) We only considers n-ary adds for now. This should be extended and
>>>> -// generalized.
>>>> +// 1) We only considers n-ary adds and muls for now. This should be extended
>>>> +// and generalized.
>>>> //
>>>> //===----------------------------------------------------------------------===//
>>>>
>>>> @@ -145,12 +145,23 @@ private:
>>>> unsigned I, Value *LHS,
>>>> Value *RHS, Type *IndexedType);
>>>>
>>>> - // Reassociate Add for better CSE.
>>>> - Instruction *tryReassociateAdd(BinaryOperator *I);
>>>> - // A helper function for tryReassociateAdd. LHS and RHS are explicitly passed.
>>>> - Instruction *tryReassociateAdd(Value *LHS, Value *RHS, Instruction *I);
>>>> - // Rewrites I to LHS + RHS if LHS is computed already.
>>>> - Instruction *tryReassociatedAdd(const SCEV *LHS, Value *RHS, Instruction *I);
>>>> + // Reassociate binary operators for better CSE.
>>>> + Instruction *tryReassociateBinaryOp(BinaryOperator *I);
>>>> +
>>>> + // A helper function for tryReassociateBinaryOp. LHS and RHS are explicitly
>>>> + // passed.
>>>> + Instruction *tryReassociateBinaryOp(Value *LHS, Value *RHS,
>>>> + BinaryOperator *I);
>>>> + // Rewrites I to (LHS op RHS) if LHS is computed already.
>>>> + Instruction *tryReassociatedBinaryOp(const SCEV *LHS, Value *RHS,
>>>> + BinaryOperator *I);
>>>> +
>>>> + // Tries to match Op1 and Op2 by using V.
>>>> + bool matchTernaryOp(BinaryOperator *I, Value *V, Value *&Op1, Value *&Op2);
>>>> +
>>>> + // Gets SCEV for (LHS op RHS).
>>>> + const SCEV *getBinarySCEV(BinaryOperator *I, const SCEV *LHS,
>>>> + const SCEV *RHS);
>>>>
>>>> // Returns the closest dominator of \c Dominatee that computes
>>>> // \c CandidateExpr. Returns null if not found.
>>>> @@ -219,6 +230,7 @@ static bool isPotentiallyNaryReassociabl
>>>> switch (I->getOpcode()) {
>>>> case Instruction::Add:
>>>> case Instruction::GetElementPtr:
>>>> + case Instruction::Mul:
>>>> return true;
>>>> default:
>>>> return false;
>>>> @@ -276,7 +288,8 @@ bool NaryReassociate::doOneIteration(Fun
>>>> Instruction *NaryReassociate::tryReassociate(Instruction *I) {
>>>> switch (I->getOpcode()) {
>>>> case Instruction::Add:
>>>> - return tryReassociateAdd(cast<BinaryOperator>(I));
>>>> + case Instruction::Mul:
>>>> + return tryReassociateBinaryOp(cast<BinaryOperator>(I));
>>>> case Instruction::GetElementPtr:
>>>> return tryReassociateGEP(cast<GetElementPtrInst>(I));
>>>> default:
>>>> @@ -453,49 +466,89 @@ GetElementPtrInst *NaryReassociate::tryR
>>>> return NewGEP;
>>>> }
>>>>
>>>> -Instruction *NaryReassociate::tryReassociateAdd(BinaryOperator *I) {
>>>> +Instruction *NaryReassociate::tryReassociateBinaryOp(BinaryOperator *I) {
>>>> Value *LHS = I->getOperand(0), *RHS = I->getOperand(1);
>>>> - if (auto *NewI = tryReassociateAdd(LHS, RHS, I))
>>>> + if (auto *NewI = tryReassociateBinaryOp(LHS, RHS, I))
>>>> return NewI;
>>>> - if (auto *NewI = tryReassociateAdd(RHS, LHS, I))
>>>> + if (auto *NewI = tryReassociateBinaryOp(RHS, LHS, I))
>>>> return NewI;
>>>> return nullptr;
>>>> }
>>>>
>>>> -Instruction *NaryReassociate::tryReassociateAdd(Value *LHS, Value *RHS,
>>>> - Instruction *I) {
>>>> +Instruction *NaryReassociate::tryReassociateBinaryOp(Value *LHS, Value *RHS,
>>>> + BinaryOperator *I) {
>>>> Value *A = nullptr, *B = nullptr;
>>>> - // To be conservative, we reassociate I only when it is the only user of A+B.
>>>> - if (LHS->hasOneUse() && match(LHS, m_Add(m_Value(A), m_Value(B)))) {
>>>> - // I = (A + B) + RHS
>>>> - // = (A + RHS) + B or (B + RHS) + A
>>>> + // To be conservative, we reassociate I only when it is the only user of (A op
>>>> + // B).
>>>> + if (LHS->hasOneUse() && matchTernaryOp(I, LHS, A, B)) {
>>>> + // I = (A op B) op RHS
>>>> + // = (A op RHS) op B or (B op RHS) op A
>>>> const SCEV *AExpr = SE->getSCEV(A), *BExpr = SE->getSCEV(B);
>>>> const SCEV *RHSExpr = SE->getSCEV(RHS);
>>>> if (BExpr != RHSExpr) {
>>>> - if (auto *NewI = tryReassociatedAdd(SE->getAddExpr(AExpr, RHSExpr), B, I))
>>>> + if (auto *NewI =
>>>> + tryReassociatedBinaryOp(getBinarySCEV(I, AExpr, RHSExpr), B, I))
>>>> return NewI;
>>>> }
>>>> if (AExpr != RHSExpr) {
>>>> - if (auto *NewI = tryReassociatedAdd(SE->getAddExpr(BExpr, RHSExpr), A, I))
>>>> + if (auto *NewI =
>>>> + tryReassociatedBinaryOp(getBinarySCEV(I, BExpr, RHSExpr), A, I))
>>>> return NewI;
>>>> }
>>>> }
>>>> return nullptr;
>>>> }
>>>>
>>>> -Instruction *NaryReassociate::tryReassociatedAdd(const SCEV *LHSExpr,
>>>> - Value *RHS, Instruction *I) {
>>>> +Instruction *NaryReassociate::tryReassociatedBinaryOp(const SCEV *LHSExpr,
>>>> + Value *RHS,
>>>> + BinaryOperator *I) {
>>>> // Look for the closest dominator LHS of I that computes LHSExpr, and replace
>>>> - // I with LHS + RHS.
>>>> + // I with LHS op RHS.
>>>> auto *LHS = findClosestMatchingDominator(LHSExpr, I);
>>>> if (LHS == nullptr)
>>>> return nullptr;
>>>>
>>>> - Instruction *NewI = BinaryOperator::CreateAdd(LHS, RHS, "", I);
>>>> + Instruction *NewI = nullptr;
>>>> + switch (I->getOpcode()) {
>>>> + case Instruction::Add:
>>>> + NewI = BinaryOperator::CreateAdd(LHS, RHS, "", I);
>>>> + break;
>>>> + case Instruction::Mul:
>>>> + NewI = BinaryOperator::CreateMul(LHS, RHS, "", I);
>>>> + break;
>>>> + default:
>>>> + llvm_unreachable("Unexpected instruction.");
>>>> + }
>>>> NewI->takeName(I);
>>>> return NewI;
>>>> }
>>>>
>>>> +bool NaryReassociate::matchTernaryOp(BinaryOperator *I, Value *V, Value *&Op1,
>>>> + Value *&Op2) {
>>>> + switch (I->getOpcode()) {
>>>> + case Instruction::Add:
>>>> + return match(V, m_Add(m_Value(Op1), m_Value(Op2)));
>>>> + case Instruction::Mul:
>>>> + return match(V, m_Mul(m_Value(Op1), m_Value(Op2)));
>>>> + default:
>>>> + llvm_unreachable("Unexpected instruction.");
>>>> + }
>>>> + return false;
>>>> +}
>>>> +
>>>> +const SCEV *NaryReassociate::getBinarySCEV(BinaryOperator *I, const SCEV *LHS,
>>>> + const SCEV *RHS) {
>>>> + switch (I->getOpcode()) {
>>>> + case Instruction::Add:
>>>> + return SE->getAddExpr(LHS, RHS);
>>>> + case Instruction::Mul:
>>>> + return SE->getMulExpr(LHS, RHS);
>>>> + default:
>>>> + llvm_unreachable("Unexpected instruction.");
>>>> + }
>>>> + return nullptr;
>>>> +}
>>>> +
>>>> Instruction *
>>>> NaryReassociate::findClosestMatchingDominator(const SCEV *CandidateExpr,
>>>> Instruction *Dominatee) {
>>>>
>>>> Added: llvm/trunk/test/Transforms/NaryReassociate/nary-mul.ll
>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/NaryReassociate/nary-mul.ll?rev=247705&view=auto <http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/NaryReassociate/nary-mul.ll?rev=247705&view=auto>
>>>> ==============================================================================
>>>> --- llvm/trunk/test/Transforms/NaryReassociate/nary-mul.ll (added)
>>>> +++ llvm/trunk/test/Transforms/NaryReassociate/nary-mul.ll Tue Sep 15 12:22:52 2015
>>>> @@ -0,0 +1,18 @@
>>>> +; RUN: opt < %s -nary-reassociate -S | FileCheck %s
>>>> +
>>>> +target datalayout = "e-i64:64-v16:16-v32:32-n16:32:64"
>>>> +
>>>> +declare void @foo(i32)
>>>> +
>>>> +; CHECK-LABEL: @bar(
>>>> +define void @bar(i32 %a, i32 %b, i32 %c) {
>>>> + %1 = mul i32 %a, %c
>>>> +; CHECK: [[BASE:%[a-zA-Z0-9]+]] = mul i32 %a, %c
>>>> + call void @foo(i32 %1)
>>>> + %2 = mul i32 %a, %b
>>>> + %3 = mul i32 %2, %c
>>>> +; CHECK: mul i32 [[BASE]], %b
>>>> + call void @foo(i32 %3)
>>>
>>> The CHECK seems incomplete to me, it is not clear what sequence is fed to the call in the end.
>>>
>>> —
>>> Mehdi
>>>
>>> _______________________________________________
>>> 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/20150915/13e21195/attachment.html>
More information about the llvm-commits
mailing list