2011/5/20 Argyrios Kyrtzidis <[email protected]> > Applying clang_stringrefize_diagnostics.diff causes DiagnosticIDs.cpp to > take > 10 mins to compile with optimizations on. See > http://llvm.org/PR9956 > > -Argyrios > > On May 19, 2011, at 12:46 PM, Argyrios Kyrtzidis wrote: > > Index: tools/libclang/CIndexDiagnostic.cpp > =================================================================== > --- tools/libclang/CIndexDiagnostic.cpp (révision 131339) > +++ tools/libclang/CIndexDiagnostic.cpp (copie de travail) > @@ -220,7 +220,8 @@ > return createCXString(""); > > unsigned ID = StoredDiag->Diag.getID(); > - if (const char *Option = DiagnosticIDs::getWarningOptionForDiag(ID)) { > + llvm::StringRef Option = DiagnosticIDs::getWarningOptionForDiag(ID); > + if (Option.data()) { > if (Disable) > > > Isn't it a bit better and more descriptive if you used !.empty() instead of > .data() ? This applies to the rest of the diffs. >
Changed, it is indeed better, especially since could end up with a valid
pointer but a 0 length.
>
> // -Wsystem-headers is a special case, not driven by the option table.
> It
> // cannot be controlled with -Werror.
> - if (OptEnd-OptStart == 14 && memcmp(OptStart, "system-headers", 14) ==
> 0) {
> + if (Opt == "system-headers") {
> Diags.setSuppressSystemWarnings(!isPositive);
>
>
> 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 ?
>
>
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.
> Thanks for your work!
>
> -Argyrios
>
>
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.
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).
This seems to have solved the performance issue:
[ 98%] Building CXX object
tools/clang/lib/Basic/CMakeFiles/clangBasic.dir/DiagnosticIDs.cpp.obj
Linking CXX static library ../../../../lib/libclangBasic.a
[100%] Built target clangBasic
real 2m37.453s
user 0m33.109s
sys 0m36.482s
(that is for the whole of libclangBasic.a and its dependencies, with -j 1,
and only DiagnosticIDs.cpp being changed)
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.
It means that both patches (to llvm and clang) shall be applied / fallen
back together.
Tested in both Debug and Release, this time :)
-- Matthieu
llvm_stringrefize_diagnostics.diff
Description: Binary data
clang_stringrefize_diagnostics.diff
Description: Binary data
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
