[llvm-commits] [llvm] r163164 - /llvm/trunk/lib/Transforms/Utils/BypassSlowDivision.cpp

Chandler Carruth chandlerc at google.com
Tue Sep 4 13:55:42 PDT 2012


On Tue, Sep 4, 2012 at 4:48 PM, Jakub Staszak <kubastaszak at gmail.com> wrote:

> Author: kuba
> Date: Tue Sep  4 15:48:24 2012
> New Revision: 163164
>
> URL: http://llvm.org/viewvc/llvm-project?rev=163164&view=rev
> Log:
> Return false if BypassSlowDivision doesn't change anything.
> Also a few minor changes:

- use pre-inc instead of post-inc
> - use isa instead of dyn_cast
> - 80 col
> - trailing spaces
>

You've mixed these into one commit, so now I'm confused -- does the first
one change behavior? It seems like it should, but maybe it was just a pure
refactoring? (If it does, test case?)


>
> Modified:
>     llvm/trunk/lib/Transforms/Utils/BypassSlowDivision.cpp
>
> Modified: llvm/trunk/lib/Transforms/Utils/BypassSlowDivision.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/BypassSlowDivision.cpp?rev=163164&r1=163163&r2=163164&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Utils/BypassSlowDivision.cpp (original)
> +++ llvm/trunk/lib/Transforms/Utils/BypassSlowDivision.cpp Tue Sep  4
> 15:48:24 2012
> @@ -61,7 +61,7 @@
>      static unsigned getHashValue(const DivOpInfo &Val) {
>        return (unsigned)(reinterpret_cast<uintptr_t>(Val.Dividend) ^
>                          reinterpret_cast<uintptr_t>(Val.Divisor)) ^
> -                       (unsigned)Val.SignedOp;
> +                        (unsigned)Val.SignedOp;
>      }
>    };
>
> @@ -71,31 +71,29 @@
>  // insertFastDiv - Substitutes the div/rem instruction with code that
> checks the
>  // value of the operands and uses a shorter-faster div/rem instruction
> when
>  // possible and the longer-slower div/rem instruction otherwise.
> -static void insertFastDiv(Function &F,
> +static bool insertFastDiv(Function &F,
>                            Function::iterator &I,
>                            BasicBlock::iterator &J,
>                            IntegerType *BypassType,
>                            bool UseDivOp,
>                            bool UseSignedOp,
> -                          DivCacheTy &PerBBDivCache)
> -{
> +                          DivCacheTy &PerBBDivCache) {
>    // Get instruction operands
>    Instruction *Instr = J;
>    Value *Dividend = Instr->getOperand(0);
>    Value *Divisor = Instr->getOperand(1);
>
> -  if (dyn_cast<ConstantInt>(Divisor) != 0 ||
> -      (dyn_cast<ConstantInt>(Dividend) != 0 &&
> -       dyn_cast<ConstantInt>(Divisor) != 0)) {
> +  if (isa<ConstantInt>(Divisor) ||
> +      (isa<ConstantInt>(Dividend) && isa<ConstantInt>(Divisor))) {
>      // Operations with immediate values should have
>      // been solved and replaced during compile time.
> -    return;
> +    return false;
>    }
>
>    // Basic Block is split before divide
>    BasicBlock *MainBB = I;
>    BasicBlock *SuccessorBB = I->splitBasicBlock(J);
> -  I++; //advance iterator I to successorBB
> +  ++I; //advance iterator I to successorBB
>
>    // Add new basic block for slow divide operation
>    BasicBlock *SlowBB = BasicBlock::Create(F.getContext(), "",
> @@ -118,17 +116,19 @@
>                                            MainBB->getParent(),
> SuccessorBB);
>    FastBB->moveBefore(SlowBB);
>    IRBuilder<> FastBuilder(FastBB, FastBB->begin());
> -  Value *ShortDivisorV = FastBuilder.CreateCast(Instruction::Trunc,
> Divisor, BypassType);
> -  Value *ShortDividendV = FastBuilder.CreateCast(Instruction::Trunc,
> Dividend, BypassType);
> +  Value *ShortDivisorV = FastBuilder.CreateCast(Instruction::Trunc,
> Divisor,
> +                                                BypassType);
> +  Value *ShortDividendV = FastBuilder.CreateCast(Instruction::Trunc,
> Dividend,
> +                                                 BypassType);
>
>    // udiv/urem because optimization only handles positive numbers
>    Value *ShortQuotientV = FastBuilder.CreateExactUDiv(ShortDividendV,
> -                                                       ShortDivisorV);
> +                                                      ShortDivisorV);
>    Value *ShortRemainderV = FastBuilder.CreateURem(ShortDividendV,
>                                                    ShortDivisorV);
>    Value *FastQuotientV = FastBuilder.CreateCast(Instruction::ZExt,
> -                                                 ShortQuotientV,
> -                                                 Dividend->getType());
> +                                                ShortQuotientV,
> +                                                Dividend->getType());
>    Value *FastRemainderV = FastBuilder.CreateCast(Instruction::ZExt,
>                                                   ShortRemainderV,
>                                                   Dividend->getType());
> @@ -144,11 +144,10 @@
>    RemPhi->addIncoming(FastRemainderV, FastBB);
>
>    // Replace Instr with appropriate phi node
> -  if (UseDivOp) {
> +  if (UseDivOp)
>      Instr->replaceAllUsesWith(QuoPhi);
> -  } else {
> +  else
>      Instr->replaceAllUsesWith(RemPhi);
> -  }
>    Instr->eraseFromParent();
>
>    // Combine operands into a single value with OR for value testing below
> @@ -174,19 +173,19 @@
>    DivOpInfo Key(UseSignedOp, Dividend, Divisor);
>    DivPhiNodes Value(QuoPhi, RemPhi);
>    PerBBDivCache.insert(std::pair<DivOpInfo, DivPhiNodes>(Key, Value));
> +  return true;
>  }
>
>  // reuseOrInsertFastDiv - Reuses previously computed dividend or
> remainder if
>  // operands and operation are identical. Otherwise call insertFastDiv to
> perform
>  // the optimization and cache the resulting dividend and remainder.
> -static void reuseOrInsertFastDiv(Function &F,
> +static bool reuseOrInsertFastDiv(Function &F,
>                                   Function::iterator &I,
>                                   BasicBlock::iterator &J,
>                                   IntegerType *BypassType,
>                                   bool UseDivOp,
>                                   bool UseSignedOp,
> -                                 DivCacheTy &PerBBDivCache)
> -{
> +                                 DivCacheTy &PerBBDivCache) {
>    // Get instruction operands
>    Instruction *Instr = J;
>    DivOpInfo Key(UseSignedOp, Instr->getOperand(0), Instr->getOperand(1));
> @@ -194,8 +193,8 @@
>
>    if (CacheI == PerBBDivCache.end()) {
>      // If previous instance does not exist, insert fast div
> -    insertFastDiv(F, I, J, BypassType, UseDivOp, UseSignedOp,
> PerBBDivCache);
> -    return;
> +    return insertFastDiv(F, I, J, BypassType, UseDivOp, UseSignedOp,
> +                         PerBBDivCache);
>    }
>
>    // Replace operation value with previously generated phi node
> @@ -209,18 +208,18 @@
>    }
>
>    // Advance to next operation
> -  J++;
> +  ++J;
>
>    // Remove redundant operation
>    Instr->eraseFromParent();
> +  return true;
>  }
>
>  // bypassSlowDivision - This optimization identifies DIV instructions
> that can
>  // be profitably bypassed and carried out with a shorter, faster divide.
>  bool bypassSlowDivision(Function &F,
>                          Function::iterator &I,
> -                        const llvm::DenseMap<Type *, Type *>
> &BypassTypeMap)
> -{
> +                        const llvm::DenseMap<Type *, Type *>
> &BypassTypeMap) {
>    DivCacheTy DivCache;
>
>    bool MadeChange = false;
> @@ -230,20 +229,22 @@
>      unsigned Opcode = J->getOpcode();
>      bool UseDivOp = Opcode == Instruction::SDiv || Opcode ==
> Instruction::UDiv;
>      bool UseRemOp = Opcode == Instruction::SRem || Opcode ==
> Instruction::URem;
> -    bool UseSignedOp = Opcode == Instruction::SDiv || Opcode ==
> Instruction::SRem;
> +    bool UseSignedOp = Opcode == Instruction::SDiv ||
> +                       Opcode == Instruction::SRem;
>
>      // Only optimize div or rem ops
> -    if (!UseDivOp && !UseRemOp) {
> +    if (!UseDivOp && !UseRemOp)
>        continue;
> -    }
> +
>      // Continue if div/rem type is not bypassed
> -    DenseMap<Type *, Type *>::const_iterator BT =
> BypassTypeMap.find(J->getType());
> -    if (BT == BypassTypeMap.end()) {
> +    DenseMap<Type *, Type *>::const_iterator BT =
> +      BypassTypeMap.find(J->getType());
> +    if (BT == BypassTypeMap.end())
>        continue;
> -    }
>
> -    IntegerType *BypassType = (IntegerType *)BT->second;
> -    reuseOrInsertFastDiv(F, I, J, BypassType, UseDivOp, UseSignedOp,
> DivCache);
> +    IntegerType *BypassType = cast<IntegerType>(BT->second);
> +    MadeChange |= reuseOrInsertFastDiv(F, I, J, BypassType, UseDivOp,
> +                                       UseSignedOp, DivCache);
>      MadeChange = true;
>    }
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120904/36f98fe8/attachment.html>


More information about the llvm-commits mailing list