[llvm] r219123 - DbgValueHistoryCalculator: Store modified registers in a BitVector instead of std::set.

[Frédéric Riss]

> On Oct 6, 2014, at 8:31 AM, Benjamin Kramer <benny.kra at googlemail.com> wrote:
> 
> Author: d0k
> Date: Mon Oct  6 10:31:04 2014
> New Revision: 219123
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=219123&view=rev
> Log:
> DbgValueHistoryCalculator: Store modified registers in a BitVector instead of std::set.
> 
> And iterate over the smaller map instead of the larger set first.  Reduces the time spent in
> calculateDbgValueHistory by 30-40%.

Cool! Do you have a specific testcase where this shows up? how did you measure it? When I last worked on that code, I tried the BitVector approach and it didn’t give me any gain on my specific testcase. I’ll try to run my testcase again.

> Modified:
>    llvm/trunk/lib/CodeGen/AsmPrinter/DbgValueHistoryCalculator.cpp
> 
> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DbgValueHistoryCalculator.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DbgValueHistoryCalculator.cpp?rev=219123&r1=219122&r2=219123&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/AsmPrinter/DbgValueHistoryCalculator.cpp (original)
> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DbgValueHistoryCalculator.cpp Mon Oct  6 10:31:04 2014
> @@ -8,6 +8,7 @@
> //===----------------------------------------------------------------------===//
> 
> #include "DbgValueHistoryCalculator.h"
> +#include "llvm/ADT/BitVector.h"
> #include "llvm/ADT/SmallVector.h"
> #include "llvm/CodeGen/MachineBasicBlock.h"
> #include "llvm/CodeGen/MachineFunction.h"
> @@ -16,12 +17,10 @@
> #include "llvm/Target/TargetRegisterInfo.h"
> #include <algorithm>
> #include <map>
> -#include <set>
> +using namespace llvm;
> 
> #define DEBUG_TYPE "dwarfdebug"
> 
> -namespace llvm {
> -
> // \brief If @MI is a DBG_VALUE with debug value described by a
> // defined register, returns the number of this register.
> // In the other case, returns 0.
> @@ -97,6 +96,19 @@ static void addRegDescribedVar(RegDescri
>   VarSet.push_back(Var);
> }
> 
> +// \brief Terminate the location range for variables described by register at
> +// @I by inserting @ClobberingInstr to their history.
> +static void clobberRegisterUses(RegDescribedVarsMap &RegVars,
> +                                RegDescribedVarsMap::iterator I,
> +                                DbgValueHistoryMap &HistMap,
> +                                const MachineInstr &ClobberingInstr) {
> +  // Iterate over all variables described by this register and add this
> +  // instruction to their history, clobbering it.
> +  for (const auto &Var : I->second)
> +    HistMap.endInstrRange(Var, ClobberingInstr);
> +  RegVars.erase(I);
> +}
> +
> // \brief Terminate the location range for variables described by register
> // @RegNo by inserting @ClobberingInstr to their history.
> static void clobberRegisterUses(RegDescribedVarsMap &RegVars, unsigned RegNo,
> @@ -105,11 +117,7 @@ static void clobberRegisterUses(RegDescr
>   const auto &I = RegVars.find(RegNo);
>   if (I == RegVars.end())
>     return;
> -  // Iterate over all variables described by this register and add this
> -  // instruction to their history, clobbering it.
> -  for (const auto &Var : I->second)
> -    HistMap.endInstrRange(Var, ClobberingInstr);
> -  RegVars.erase(I);
> +  clobberRegisterUses(RegVars, I, HistMap, ClobberingInstr);
> }
> 
> // \brief Collect all registers clobbered by @MI and apply the functor
> @@ -142,11 +150,12 @@ static const MachineInstr *getFirstEpilo
>   // as the return instruction.
>   DebugLoc LastLoc = LastMI->getDebugLoc();
>   auto Res = LastMI;
> -  for (MachineBasicBlock::const_reverse_iterator I(std::next(LastMI)); I != MBB.rend();
> -       ++I) {
> +  for (MachineBasicBlock::const_reverse_iterator I(std::next(LastMI)),
> +       E = MBB.rend();
> +       I != E; ++I) {
>     if (I->getDebugLoc() != LastLoc)
>       return Res;
> -    Res = std::prev(I.base());
> +    Res = &*I;
>   }
>   // If all instructions have the same debug location, assume whole MBB is
>   // an epilogue.
> @@ -157,7 +166,7 @@ static const MachineInstr *getFirstEpilo
> // contents is changed outside of the prologue and epilogue).
> static void collectChangingRegs(const MachineFunction *MF,
>                                 const TargetRegisterInfo *TRI,
> -                                std::set<unsigned> &Regs) {
> +                                BitVector &Regs) {
>   for (const auto &MBB : *MF) {
>     auto FirstEpilogueInst = getFirstEpilogueInst(MBB);
> 
> @@ -165,15 +174,15 @@ static void collectChangingRegs(const Ma
>       if (&MI == FirstEpilogueInst)
>         break;
>       if (!MI.getFlag(MachineInstr::FrameSetup))
> -        applyToClobberedRegisters(MI, TRI, [&](unsigned r) { Regs.insert(r); });
> +        applyToClobberedRegisters(MI, TRI, [&](unsigned r) { Regs.set(r); });
>     }
>   }
> }
> 
> -void calculateDbgValueHistory(const MachineFunction *MF,
> -                              const TargetRegisterInfo *TRI,
> -                              DbgValueHistoryMap &Result) {
> -  std::set<unsigned> ChangingRegs;
> +void llvm::calculateDbgValueHistory(const MachineFunction *MF,
> +                                    const TargetRegisterInfo *TRI,
> +                                    DbgValueHistoryMap &Result) {
> +  BitVector ChangingRegs(TRI->getNumRegs());
>   collectChangingRegs(MF, TRI, ChangingRegs);
> 
>   RegDescribedVarsMap RegVars;
> @@ -183,7 +192,7 @@ void calculateDbgValueHistory(const Mach
>         // Not a DBG_VALUE instruction. It may clobber registers which describe
>         // some variables.
>         applyToClobberedRegisters(MI, TRI, [&](unsigned RegNo) {
> -          if (ChangingRegs.count(RegNo))
> +          if (ChangingRegs.test(RegNo))
>             clobberRegisterUses(RegVars, RegNo, Result, MI);
>         });
>         continue;
> @@ -207,11 +216,12 @@ void calculateDbgValueHistory(const Mach
>     // Make sure locations for register-described variables are valid only
>     // until the end of the basic block (unless it's the last basic block, in
>     // which case let their liveness run off to the end of the function).
> -    if (!MBB.empty() &&  &MBB != &MF->back()) {
> -      for (unsigned RegNo : ChangingRegs)
> -        clobberRegisterUses(RegVars, RegNo, Result, MBB.back());
> +    if (!MBB.empty() && &MBB != &MF->back()) {
> +      for (auto I = RegVars.begin(), E = RegVars.end(); I != E;) {
> +        auto CurElem = I++; // CurElem can be erased below.
> +        if (ChangingRegs.test(CurElem->first))
> +          clobberRegisterUses(RegVars, CurElem, Result, MBB.back());
> +      }
>     }
>   }
> }
> -
> -}
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits