<div dir="ltr">Ah, yes that is useful, but it's not on by default. This patch should probably also improve release build performance.</div><div class="gmail_extra"><br clear="all"><div><div class="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div dir="ltr">Sincerely,<br>Alexander Riccio<br>--<br>"Change the world or go home."<div><a href="http://about.me/ariccio" target="_blank">about.me/ariccio</a></div><div><a href="http://about.me/ariccio" target="_blank"><br></a></div><div>If left to my own devices, I will build more.</div><div>⁂<br></div></div></div></div></div></div></div>
<br><div class="gmail_quote">On Tue, Feb 2, 2016 at 8:50 PM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span class="">On Tue, Feb 2, 2016 at 5:35 PM, Alexander Riccio via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">ariccio created this revision.<br>
ariccio added reviewers: rnk, qcolombet, craig.topper.<br>
ariccio added a subscriber: llvm-commits.<br>
<br>
This patch adds a reserve call to an expensive function (`llvm::LoadIntrinsics`), and may fix a few other low hanging performance fruit (I've put them in comments for now, so we can discuss).<br>
<br>
**Motivation:**<br>
<br>
As I'm sure other developers do, when I build LLVM, I build the entire project with the same config (`Debug`, `MinSizeRel`, `Release`, or `RelWithDebInfo`). However, the `Debug` config also builds llvm-tblgen in `Debug` mode. Later build steps that run llvm-tblgen then can actually be the slowest steps in the entire build. Nobody likes slow builds.<br></blockquote><div><br></div></span><div>FWIW, there's a CMake config flag to address this: LLVM_OPTIMIZED_TABLEGEN ("Force TableGen to be built with optimization")</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5">
<br>
<br>
<br>
<a href="http://reviews.llvm.org/D16832" rel="noreferrer" target="_blank">http://reviews.llvm.org/D16832</a><br>
<br>
Files:<br>
C:/LLVM/llvm/include/llvm/TableGen/Record.h<br>
C:/LLVM/llvm/utils/TableGen/CodeGenInstruction.cpp<br>
C:/LLVM/llvm/utils/TableGen/CodeGenTarget.cpp<br>
<br>
Index: C:/LLVM/llvm/utils/TableGen/CodeGenInstruction.cpp<br>
===================================================================<br>
--- C:/LLVM/llvm/utils/TableGen/CodeGenInstruction.cpp<br>
+++ C:/LLVM/llvm/utils/TableGen/CodeGenInstruction.cpp<br>
@@ -49,6 +49,11 @@<br>
<br>
unsigned MIOperandNo = 0;<br>
std::set<std::string> OperandNames;<br>
+<br>
+ // TODO: can we reserve space for e elements in OperandList?<br>
+ // Profiling a debug build with ETW revealed that the emplace_back<br>
+ // at the very bottom of this loop can take up to 227ms, largely<br>
+ // spent in reallocation.<br>
for (unsigned i = 0, e = InDI->getNumArgs()+OutDI->getNumArgs(); i != e; ++i){<br>
Init *ArgInit;<br>
std::string ArgName;<br>
Index: C:/LLVM/llvm/include/llvm/TableGen/Record.h<br>
===================================================================<br>
--- C:/LLVM/llvm/include/llvm/TableGen/Record.h<br>
+++ C:/LLVM/llvm/include/llvm/TableGen/Record.h<br>
@@ -1307,9 +1307,12 @@<br>
}<br>
<br>
bool isSubClassOf(StringRef Name) const {<br>
- for (const auto &SCPair : SuperClasses)<br>
+ for (const auto &SCPair : SuperClasses) {<br>
+ // TODO: getNameInitAsString copy constructs a new std::string,<br>
+ // yet we're only using it in this comparison. Is there a better way?<br>
if (SCPair.first->getNameInitAsString() == Name)<br>
return true;<br>
+ }<br>
return false;<br>
}<br>
<br>
Index: C:/LLVM/llvm/utils/TableGen/CodeGenTarget.cpp<br>
===================================================================<br>
--- C:/LLVM/llvm/utils/TableGen/CodeGenTarget.cpp<br>
+++ C:/LLVM/llvm/utils/TableGen/CodeGenTarget.cpp<br>
@@ -441,12 +441,24 @@<br>
std::vector<Record*> I = RC.getAllDerivedDefinitions("Intrinsic");<br>
<br>
std::vector<CodeGenIntrinsic> Result;<br>
+<br>
+ // Allocate enough memory to hold enough for worst case scenario<br>
+ // in the following loop. Profiling a debug build with ETW<br>
+ // revealed that push_back in the loop could take up to 280ms,<br>
+ // largely spent in reallocation.<br>
+ Result.reserve(I.size());<br>
<br>
for (unsigned i = 0, e = I.size(); i != e; ++i) {<br>
bool isTarget = I[i]->getValueAsBit("isTarget");<br>
if (isTarget == TargetOnly)<br>
Result.push_back(CodeGenIntrinsic(I[i]));<br>
}<br>
+<br>
+ // TODO: does this lambda need to accept parameters by value?<br>
+ // Profiling a debug build with ETW revealed that each call of<br>
+ // std::_Unguarded_partition (an internal function, called by std::sort),<br>
+ // can spend hundreds of milliseconds inside this lambda calling<br>
+ // ~CodeGenIntrinsic(), which is quite wasteful.<br>
std::sort(Result.begin(), Result.end(),<br>
[](CodeGenIntrinsic LHS, CodeGenIntrinsic RHS) {<br>
return LHS.Name < RHS.Name;<br>
<br>
<br>
<br></div></div>_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
<br></blockquote></div><br></div></div>
</blockquote></div><br></div>