[llvm] r247705 - [NaryReassociate] Add support for Mul instructions
Mehdi Amini via llvm-commits
llvm-commits at lists.llvm.org
Tue Sep 15 11:07:10 PDT 2015
> On Sep 15, 2015, at 11:03 AM, Marcello Maggioni <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/2fd2ca4d/attachment.html>
More information about the llvm-commits
mailing list