[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