<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Oct 6, 2014 at 10:09 AM, Benjamin Kramer <span dir="ltr"><<a href="mailto:benny.kra@gmail.com" target="_blank">benny.kra@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class=""><br>
On 06.10.2014, at 18:29, Frédéric Riss <<a href="mailto:friss@apple.com">friss@apple.com</a>> wrote:<br>
<br>
><br>
>> On Oct 6, 2014, at 8:31 AM, Benjamin Kramer <<a href="mailto:benny.kra@googlemail.com">benny.kra@googlemail.com</a>> wrote:<br>
>><br>
>> Author: d0k<br>
>> Date: Mon Oct  6 10:31:04 2014<br>
>> New Revision: 219123<br>
>><br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=219123&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=219123&view=rev</a><br>
>> Log:<br>
>> DbgValueHistoryCalculator: Store modified registers in a BitVector instead of std::set.<br>
>><br>
>> And iterate over the smaller map instead of the larger set first.  Reduces the time spent in<br>
>> calculateDbgValueHistory by 30-40%.<br>
><br>
> 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.<br>
<br>
</span>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 ;)<br></blockquote><div><br></div><div>Thanks for the change!</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
- Ben<br>
<div class="HOEnZb"><div class="h5"><br>
><br>
>> Modified:<br>
>>   llvm/trunk/lib/CodeGen/AsmPrinter/DbgValueHistoryCalculator.cpp<br>
>><br>
>> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DbgValueHistoryCalculator.cpp<br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DbgValueHistoryCalculator.cpp?rev=219123&r1=219122&r2=219123&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DbgValueHistoryCalculator.cpp?rev=219123&r1=219122&r2=219123&view=diff</a><br>
>> ==============================================================================<br>
>> --- llvm/trunk/lib/CodeGen/AsmPrinter/DbgValueHistoryCalculator.cpp (original)<br>
>> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DbgValueHistoryCalculator.cpp Mon Oct  6 10:31:04 2014<br>
>> @@ -8,6 +8,7 @@<br>
>> //===----------------------------------------------------------------------===//<br>
>><br>
>> #include "DbgValueHistoryCalculator.h"<br>
>> +#include "llvm/ADT/BitVector.h"<br>
>> #include "llvm/ADT/SmallVector.h"<br>
>> #include "llvm/CodeGen/MachineBasicBlock.h"<br>
>> #include "llvm/CodeGen/MachineFunction.h"<br>
>> @@ -16,12 +17,10 @@<br>
>> #include "llvm/Target/TargetRegisterInfo.h"<br>
>> #include <algorithm><br>
>> #include <map><br>
>> -#include <set><br>
>> +using namespace llvm;<br>
>><br>
>> #define DEBUG_TYPE "dwarfdebug"<br>
>><br>
>> -namespace llvm {<br>
>> -<br>
>> // \brief If @MI is a DBG_VALUE with debug value described by a<br>
>> // defined register, returns the number of this register.<br>
>> // In the other case, returns 0.<br>
>> @@ -97,6 +96,19 @@ static void addRegDescribedVar(RegDescri<br>
>>  VarSet.push_back(Var);<br>
>> }<br>
>><br>
>> +// \brief Terminate the location range for variables described by register at<br>
>> +// @I by inserting @ClobberingInstr to their history.<br>
>> +static void clobberRegisterUses(RegDescribedVarsMap &RegVars,<br>
>> +                                RegDescribedVarsMap::iterator I,<br>
>> +                                DbgValueHistoryMap &HistMap,<br>
>> +                                const MachineInstr &ClobberingInstr) {<br>
>> +  // Iterate over all variables described by this register and add this<br>
>> +  // instruction to their history, clobbering it.<br>
>> +  for (const auto &Var : I->second)<br>
>> +    HistMap.endInstrRange(Var, ClobberingInstr);<br>
>> +  RegVars.erase(I);<br>
>> +}<br>
>> +<br>
>> // \brief Terminate the location range for variables described by register<br>
>> // @RegNo by inserting @ClobberingInstr to their history.<br>
>> static void clobberRegisterUses(RegDescribedVarsMap &RegVars, unsigned RegNo,<br>
>> @@ -105,11 +117,7 @@ static void clobberRegisterUses(RegDescr<br>
>>  const auto &I = RegVars.find(RegNo);<br>
>>  if (I == RegVars.end())<br>
>>    return;<br>
>> -  // Iterate over all variables described by this register and add this<br>
>> -  // instruction to their history, clobbering it.<br>
>> -  for (const auto &Var : I->second)<br>
>> -    HistMap.endInstrRange(Var, ClobberingInstr);<br>
>> -  RegVars.erase(I);<br>
>> +  clobberRegisterUses(RegVars, I, HistMap, ClobberingInstr);<br>
>> }<br>
>><br>
>> // \brief Collect all registers clobbered by @MI and apply the functor<br>
>> @@ -142,11 +150,12 @@ static const MachineInstr *getFirstEpilo<br>
>>  // as the return instruction.<br>
>>  DebugLoc LastLoc = LastMI->getDebugLoc();<br>
>>  auto Res = LastMI;<br>
>> -  for (MachineBasicBlock::const_reverse_iterator I(std::next(LastMI)); I != MBB.rend();<br>
>> -       ++I) {<br>
>> +  for (MachineBasicBlock::const_reverse_iterator I(std::next(LastMI)),<br>
>> +       E = MBB.rend();<br>
>> +       I != E; ++I) {<br>
>>    if (I->getDebugLoc() != LastLoc)<br>
>>      return Res;<br>
>> -    Res = std::prev(I.base());<br>
>> +    Res = &*I;<br>
>>  }<br>
>>  // If all instructions have the same debug location, assume whole MBB is<br>
>>  // an epilogue.<br>
>> @@ -157,7 +166,7 @@ static const MachineInstr *getFirstEpilo<br>
>> // contents is changed outside of the prologue and epilogue).<br>
>> static void collectChangingRegs(const MachineFunction *MF,<br>
>>                                const TargetRegisterInfo *TRI,<br>
>> -                                std::set<unsigned> &Regs) {<br>
>> +                                BitVector &Regs) {<br>
>>  for (const auto &MBB : *MF) {<br>
>>    auto FirstEpilogueInst = getFirstEpilogueInst(MBB);<br>
>><br>
>> @@ -165,15 +174,15 @@ static void collectChangingRegs(const Ma<br>
>>      if (&MI == FirstEpilogueInst)<br>
>>        break;<br>
>>      if (!MI.getFlag(MachineInstr::FrameSetup))<br>
>> -        applyToClobberedRegisters(MI, TRI, [&](unsigned r) { Regs.insert(r); });<br>
>> +        applyToClobberedRegisters(MI, TRI, [&](unsigned r) { Regs.set(r); });<br>
>>    }<br>
>>  }<br>
>> }<br>
>><br>
>> -void calculateDbgValueHistory(const MachineFunction *MF,<br>
>> -                              const TargetRegisterInfo *TRI,<br>
>> -                              DbgValueHistoryMap &Result) {<br>
>> -  std::set<unsigned> ChangingRegs;<br>
>> +void llvm::calculateDbgValueHistory(const MachineFunction *MF,<br>
>> +                                    const TargetRegisterInfo *TRI,<br>
>> +                                    DbgValueHistoryMap &Result) {<br>
>> +  BitVector ChangingRegs(TRI->getNumRegs());<br>
>>  collectChangingRegs(MF, TRI, ChangingRegs);<br>
>><br>
>>  RegDescribedVarsMap RegVars;<br>
>> @@ -183,7 +192,7 @@ void calculateDbgValueHistory(const Mach<br>
>>        // Not a DBG_VALUE instruction. It may clobber registers which describe<br>
>>        // some variables.<br>
>>        applyToClobberedRegisters(MI, TRI, [&](unsigned RegNo) {<br>
>> -          if (ChangingRegs.count(RegNo))<br>
>> +          if (ChangingRegs.test(RegNo))<br>
>>            clobberRegisterUses(RegVars, RegNo, Result, MI);<br>
>>        });<br>
>>        continue;<br>
>> @@ -207,11 +216,12 @@ void calculateDbgValueHistory(const Mach<br>
>>    // Make sure locations for register-described variables are valid only<br>
>>    // until the end of the basic block (unless it's the last basic block, in<br>
>>    // which case let their liveness run off to the end of the function).<br>
>> -    if (!MBB.empty() &&  &MBB != &MF->back()) {<br>
>> -      for (unsigned RegNo : ChangingRegs)<br>
>> -        clobberRegisterUses(RegVars, RegNo, Result, MBB.back());<br>
>> +    if (!MBB.empty() && &MBB != &MF->back()) {<br>
>> +      for (auto I = RegVars.begin(), E = RegVars.end(); I != E;) {<br>
>> +        auto CurElem = I++; // CurElem can be erased below.<br>
>> +        if (ChangingRegs.test(CurElem->first))<br>
>> +          clobberRegisterUses(RegVars, CurElem, Result, MBB.back());<br>
>> +      }<br>
>>    }<br>
>>  }<br>
>> }<br>
>> -<br>
>> -}<br>
>><br>
>><br>
>> _______________________________________________<br>
>> llvm-commits mailing list<br>
>> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
>> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
><br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div dir="ltr">Alexey Samsonov<br><a href="mailto:vonosmas@gmail.com" target="_blank">vonosmas@gmail.com</a></div>
</div></div>