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

Benjamin Kramer benny.kra at gmail.com
Mon Oct 6 10:09:52 PDT 2014


On 06.10.2014, at 18:29, Frédéric Riss <friss at apple.com> wrote:

> 
>> 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.

This was on a large-ish .c test case with clang -O2 -g under a profiler. This function wasn't particularly hot but std::set always catches my eye ;)

- Ben

> 
>> 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
> 





More information about the llvm-commits mailing list