[llvm] r247705 - [NaryReassociate] Add support for Mul instructions
Marcello Maggioni via llvm-commits
llvm-commits at lists.llvm.org
Tue Sep 15 11:03:00 PDT 2015
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?
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> wrote:
>
>>
>> On Sep 15, 2015, at 10:22 AM, Marcello Maggioni via llvm-commits <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
>> 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)
>>
>> 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
>> ==============================================================================
>> --- 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
>> ==============================================================================
>> --- 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/a7d303c4/attachment-0001.html>
More information about the llvm-commits
mailing list