<br><br><div class="gmail_quote">2011/5/20 Argyrios Kyrtzidis <span dir="ltr"><<a href="mailto:kyrtzidis@apple.com">kyrtzidis@apple.com</a>></span><br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<div style="word-wrap:break-word"><div>Applying clang_stringrefize_diagnostics.diff causes DiagnosticIDs.cpp to take > 10 mins to compile with optimizations on. See <a href="http://llvm.org/PR9956" target="_blank">http://llvm.org/PR9956</a></div>
<div><br></div><div>-Argyrios</div><div><br></div><div><div><div><div></div><div style="word-wrap:break-word"><div>On May 19, 2011, at 12:46 PM, Argyrios Kyrtzidis wrote:<blockquote type="cite"><div><font>Index: tools/libclang/CIndexDiagnostic.cpp</font></div>
<div><font>===================================================================</font></div><div><font>--- tools/libclang/CIndexDiagnostic.cpp</font><span style="white-space:pre-wrap"><font>   </font></span><font>(révision 131339)</font></div>
<div><font>+++ tools/libclang/CIndexDiagnostic.cpp</font><span style="white-space:pre-wrap"><font>      </font></span><font>(copie de travail)</font></div><div><font>@@ -220,7 +220,8 @@</font></div><div><font>     return createCXString("");</font></div>
<div><font>   </font></div><div><font>   unsigned ID = StoredDiag->Diag.getID();</font></div><div><font>-  if (const char *Option = DiagnosticIDs::getWarningOptionForDiag(ID)) {</font></div><div><font>+  llvm::StringRef Option = DiagnosticIDs::getWarningOptionForDiag(ID);</font></div>
<div><font>+  if (Option.data()) {</font></div><div><font>     if (Disable)</font></div></blockquote></div><div><div><br></div></div><div>Isn't it a bit better and more descriptive if you used !.empty() instead of .data() ? This applies to the rest of the diffs.</div>
</div></div></div></div></div></blockquote><div><br>Changed, it is indeed better, especially since could end up with a valid pointer but a 0 length.<br> </div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
<div style="word-wrap: break-word;"><div><div><div><div style="word-wrap: break-word;"><div><br></div><div><blockquote type="cite"><div><font>     // -Wsystem-headers is a special case, not driven by the option table.  It</font></div>
<div><font>     // cannot be controlled with -Werror.</font></div><div><font>-    if (OptEnd-OptStart == 14 && memcmp(OptStart, "system-headers", 14) == 0) {</font></div><div><font>+    if (Opt == "system-headers") {</font></div>
<div><font>       Diags.setSuppressSystemWarnings(!isPositive);</font></div></blockquote></div><div><div><br></div></div><div>Probably a not-worth-much micro-optimization but with the changes in lib/Frontend/Warnings.cpp you are introducing multiple strlen calls inside the loop, maybe use something like llvm::StringRef("system-headers", 14) or initialize the StringRefs outside the loop ?</div>
<div><br></div></div></div></div></div></div></blockquote><div> </div><div>As you noticed in a private answer, since the constructor of StringRef is inlined within the header, the call to strlen should end up being removed by the compiler and turned into a constant. Leaving the C-strings as they are makes for much more readable code. <br>
</div><div> </div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;"><div style="word-wrap: break-word;"><div><div><div><div style="word-wrap: break-word;">
<div></div><div>Thanks for your work!</div><div><br></div><div>-Argyrios</div><br></div></div></div></div></div></blockquote><div><br>The initial patch for StringRef-ization had been done without changing the code generated by TableGen. It introduced about ~10k  llvm::StringRef("xx")  statements, which I believe were the root cause of the compilation time going overboard.<br>
<br>I have reviewed my patch by completely changing the files generated by TableGen for the diagnostics, and notably generating all  StringRef  constructions either as  llvm::StringRef()  for empty strings or  llvm::StringRef("xx", 2)  for real strings (with the necessary escaping).<br>
<br>This seems to have solved the performance issue:<br><br>[ 98%] Building CXX object tools/clang/lib/Basic/CMakeFiles/clangBasic.dir/DiagnosticIDs.cpp.obj<br>Linking CXX static library ../../../../lib/libclangBasic.a<br>
[100%] Built target clangBasic<br><br>real    2m37.453s<br>user    0m33.109s<br>sys     0m36.482s<br><br>(that is for the whole of libclangBasic.a and its dependencies, with -j 1, and only DiagnosticIDs.cpp being changed)<br>
<br><br>Now, instead of generating a macro call with some arguments, I directly generate what's required for the table definitions, which is a consequent change.<br><br>It means that both patches (to llvm and clang) shall be applied / fallen back together.<br>
<br><br>Tested in both Debug and Release, this time :)<br><br>-- Matthieu<br></div></div><br>