[llvm] r247705 - [NaryReassociate] Add support for Mul instructions
Mehdi Amini via llvm-commits
llvm-commits at lists.llvm.org
Tue Sep 15 11:20:06 PDT 2015
I did $ cat nary-add.ll and looked at the last four cases in the files, which are OK. Out of luck you took example on the bad cases…
—
Mehdi
> On Sep 15, 2015, at 11:13 AM, Marcello Maggioni <mmaggioni at apple.com> wrote:
>
> 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 <mailto: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/03679c2f/attachment.html>
More information about the llvm-commits
mailing list