<div dir="ltr">This is my plan. About to commit. Please let me know if it helps.</div><br><div class="gmail_quote"><div dir="ltr">On Wed, Sep 16, 2015 at 12:53 PM Justin Bogner <<a href="mailto:mail@justinbogner.com">mail@justinbogner.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Chandler Carruth <<a href="mailto:chandlerc@gmail.com" target="_blank">chandlerc@gmail.com</a>> writes:<br>
> Sure, I hadn't realized how much this hurt debugging for you. How can we<br>
> fix it? We're in violent agreement that this is a total workaround that we<br>
> should do if there is a low cost way, not something we should jsut do<br>
> blindly.<br>
<br>
The case that's been coming up for me is basically just the "char *"<br>
constructor - calling functions with a "char *" and getting the<br>
conversion is pretty common while debugging. It seems like a reasonable<br>
trade off to leave the attribute on the non-constructor methods, which<br>
mostly just access attributes you can get at in the debugger anyway, and<br>
just drop it for the constructors. WDYT?<br>
<br>
> On Tue, Sep 15, 2015 at 8:16 PM Justin Bogner <<a href="mailto:mail@justinbogner.com" target="_blank">mail@justinbogner.com</a>> wrote:<br>
><br>
>> Chandler Carruth via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> writes:<br>
>> > Author: chandlerc<br>
>> > Date: Thu Sep 10 03:29:35 2015<br>
>> > New Revision: 247253<br>
>> ><br>
>> > URL: <a href="http://llvm.org/viewvc/llvm-project?rev=247253&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=247253&view=rev</a><br>
>> > Log:<br>
>> > [ADT] Apply a large hammer to StringRef functions: attribute<br>
>> always_inline.<br>
>> ><br>
>> > The logic of this follows something Howard does in libc++ and something<br>
>> > I discussed with Chris eons ago -- for a lot of functions, there is<br>
>> > really no benefit to preserving "debug information" by leaving the<br>
>> > out-of-line even in debug builds. This is especially true as we now do<br>
>> > a very good job of preserving most debug information even in the face of<br>
>> > inlining. There are a bunch of methods in StringRef that we are paying<br>
>> > a completely unacceptable amount for with every debug build of every<br>
>> > LLVM developer.<br>
>> ><br>
>> > Some day, we should fix Clang/LLVM so that developers can reasonable<br>
>> > use a default of something other than '-O0' and not waste their lives<br>
>> > waiting on *completely* unoptimized code to execute. We should have<br>
>> > a default that doesn't impede debugging while providing at least<br>
>> > plausable performance.<br>
>> ><br>
>> > But today is not that day.<br>
>> ><br>
>> > So today, I'm applying always_inline to the functions that are really<br>
>> > hurting the critical path for stuff like 'check_llvm'. I'm being very<br>
>> > cautious here, but there are a few other APIs that we really should do<br>
>> > this for as a matter of pragmatism. Hopefully we can rip this out some<br>
>> > day.<br>
>> ><br>
>> > With this change, TripleTest.Normalization runtime decreases by over<br>
>> > 10%, and the total 'check-llvm' time on my 48-core box goes from 38s to<br>
>> > just under 37s.<br>
>> ><br>
>> > Modified:<br>
>> > llvm/trunk/include/llvm/ADT/StringRef.h<br>
>> ><br>
>> > Modified: llvm/trunk/include/llvm/ADT/StringRef.h<br>
>> > URL:<br>
>> ><br>
>> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/StringRef.h?rev=247253&r1=247252&r2=247253&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/StringRef.h?rev=247253&r1=247252&r2=247253&view=diff</a><br>
>> ><br>
>> ==============================================================================<br>
>> ><br>
>> > --- llvm/trunk/include/llvm/ADT/StringRef.h (original)<br>
>> > +++ llvm/trunk/include/llvm/ADT/StringRef.h Thu Sep 10 03:29:35 2015<br>
>> > @@ -10,6 +10,7 @@<br>
>> > #ifndef LLVM_ADT_STRINGREF_H<br>
>> > #define LLVM_ADT_STRINGREF_H<br>
>> ><br>
>> > +#include "llvm/Support/Compiler.h"<br>
>> > #include <algorithm><br>
>> > #include <cassert><br>
>> > #include <cstring><br>
>> > @@ -53,6 +54,7 @@ namespace llvm {<br>
>> ><br>
>> > // Workaround memcmp issue with null pointers (undefined behavior)<br>
>> > // by providing a specialized version<br>
>> > + LLVM_ATTRIBUTE_ALWAYS_INLINE<br>
>> > static int compareMemory(const char *Lhs, const char *Rhs, size_t<br>
>> Length) {<br>
>> > if (Length == 0) { return 0; }<br>
>> > return ::memcmp(Lhs,Rhs,Length);<br>
>> > @@ -63,9 +65,11 @@ namespace llvm {<br>
>> > /// @{<br>
>> ><br>
>><br>
>> > /// Construct an empty string ref.<br>
>> > + LLVM_ATTRIBUTE_ALWAYS_INLINE<br>
>> > /*implicit*/ StringRef() : Data(nullptr), Length(0) {}<br>
>> ><br>
>> > /// Construct a string ref from a cstring.<br>
>> > + LLVM_ATTRIBUTE_ALWAYS_INLINE<br>
>> > /*implicit*/ StringRef(const char *Str)<br>
>> > : Data(Str) {<br>
>> > assert(Str && "StringRef cannot be built from a NULL argument");<br>
>> > @@ -73,6 +77,7 @@ namespace llvm {<br>
>> > }<br>
>> ><br>
>> > /// Construct a string ref from a pointer and length.<br>
>> > + LLVM_ATTRIBUTE_ALWAYS_INLINE<br>
>> > /*implicit*/ StringRef(const char *data, size_t length)<br>
>> > : Data(data), Length(length) {<br>
>> > assert((data || length == 0) &&<br>
>> > @@ -80,6 +85,7 @@ namespace llvm {<br>
>> > }<br>
>> ><br>
>> > /// Construct a string ref from an std::string.<br>
>> > + LLVM_ATTRIBUTE_ALWAYS_INLINE<br>
>> > /*implicit*/ StringRef(const std::string &Str)<br>
>> > : Data(Str.data()), Length(Str.length()) {}<br>
>><br>
>> This makes constructing StringRefs in lldb basically impossible, which<br>
>> means that functions in llvm that take a string for an argument are<br>
>> basically uncallable in a debugger. So, basically, this makes my life<br>
>> much worse. I'm fine with making "-O0 -g" builds faster if there isn't<br>
>> really a cost to it, but this kind of thing is just a work around for<br>
>> the fact that nobody's bothering to invest in making optimized debugging<br>
>> in llvm useable. The trade off is wrong.<br>
>><br>
>> ><br>
>> > @@ -104,12 +110,15 @@ namespace llvm {<br>
>> ><br>
>> > /// data - Get a pointer to the start of the string (which may not<br>
>> be null<br>
>> > /// terminated).<br>
>> > + LLVM_ATTRIBUTE_ALWAYS_INLINE<br>
>> > const char *data() const { return Data; }<br>
>> ><br>
>> > /// empty - Check if the string is empty.<br>
>> > + LLVM_ATTRIBUTE_ALWAYS_INLINE<br>
>> > bool empty() const { return Length == 0; }<br>
>> ><br>
>> > /// size - Get the string size.<br>
>> > + LLVM_ATTRIBUTE_ALWAYS_INLINE<br>
>> > size_t size() const { return Length; }<br>
>> ><br>
>> > /// front - Get the first character in the string.<br>
>> > @@ -133,6 +142,7 @@ namespace llvm {<br>
>> ><br>
>> > /// equals - Check for string equality, this is more efficient than<br>
>> > /// compare() when the relative ordering of inequal strings isn't<br>
>> needed.<br>
>> > + LLVM_ATTRIBUTE_ALWAYS_INLINE<br>
>> > bool equals(StringRef RHS) const {<br>
>> > return (Length == RHS.Length &&<br>
>> > compareMemory(Data, RHS.Data, RHS.Length) == 0);<br>
>> > @@ -145,6 +155,7 @@ namespace llvm {<br>
>> ><br>
>> > /// compare - Compare two strings; the result is -1, 0, or 1 if<br>
>> this string<br>
>> > /// is lexicographically less than, equal to, or greater than the<br>
>> \p RHS.<br>
>> > + LLVM_ATTRIBUTE_ALWAYS_INLINE<br>
>> > int compare(StringRef RHS) const {<br>
>> > // Check the prefix for a mismatch.<br>
>> > if (int Res = compareMemory(Data, RHS.Data, std::min(Length,<br>
>> RHS.Length)))<br>
>> > @@ -212,6 +223,7 @@ namespace llvm {<br>
>> > /// @{<br>
>> ><br>
>> > /// Check if this string starts with the given \p Prefix.<br>
>> > + LLVM_ATTRIBUTE_ALWAYS_INLINE<br>
>> > bool startswith(StringRef Prefix) const {<br>
>> > return Length >= Prefix.Length &&<br>
>> > compareMemory(Data, Prefix.Data, Prefix.Length) == 0;<br>
>> > @@ -221,6 +233,7 @@ namespace llvm {<br>
>> > bool startswith_lower(StringRef Prefix) const;<br>
>> ><br>
>> > /// Check if this string ends with the given \p Suffix.<br>
>> > + LLVM_ATTRIBUTE_ALWAYS_INLINE<br>
>> > bool endswith(StringRef Suffix) const {<br>
>> > return Length >= Suffix.Length &&<br>
>> > compareMemory(end() - Suffix.Length, Suffix.Data,<br>
>> Suffix.Length) == 0;<br>
>> > @@ -237,6 +250,7 @@ namespace llvm {<br>
>> > ///<br>
>> > /// \returns The index of the first occurrence of \p C, or npos if<br>
>> not<br>
>> > /// found.<br>
>> > + LLVM_ATTRIBUTE_ALWAYS_INLINE<br>
>> > size_t find(char C, size_t From = 0) const {<br>
>> > size_t FindBegin = std::min(From, Length);<br>
>> > if (FindBegin < Length) { // Avoid calling memchr with nullptr.<br>
>> > @@ -402,6 +416,7 @@ namespace llvm {<br>
>> > /// \param N The number of characters to included in the substring.<br>
>> If N<br>
>> > /// exceeds the number of characters remaining in the string, the<br>
>> string<br>
>> > /// suffix (starting with \p Start) will be returned.<br>
>> > + LLVM_ATTRIBUTE_ALWAYS_INLINE<br>
>> > StringRef substr(size_t Start, size_t N = npos) const {<br>
>> > Start = std::min(Start, Length);<br>
>> > return StringRef(Data + Start, std::min(N, Length - Start));<br>
>> > @@ -409,6 +424,7 @@ namespace llvm {<br>
>> ><br>
>> > /// Return a StringRef equal to 'this' but with the first \p N<br>
>> elements<br>
>> > /// dropped.<br>
>> > + LLVM_ATTRIBUTE_ALWAYS_INLINE<br>
>> > StringRef drop_front(size_t N = 1) const {<br>
>> > assert(size() >= N && "Dropping more elements than exist");<br>
>> > return substr(N);<br>
>> > @@ -416,6 +432,7 @@ namespace llvm {<br>
>> ><br>
>> > /// Return a StringRef equal to 'this' but with the last \p N<br>
>> elements<br>
>> > /// dropped.<br>
>> > + LLVM_ATTRIBUTE_ALWAYS_INLINE<br>
>> > StringRef drop_back(size_t N = 1) const {<br>
>> > assert(size() >= N && "Dropping more elements than exist");<br>
>> > return substr(0, size()-N);<br>
>> > @@ -431,6 +448,7 @@ namespace llvm {<br>
>> > /// substring. If this is npos, or less than \p Start, or exceeds<br>
>> the<br>
>> > /// number of characters remaining in the string, the string suffix<br>
>> > /// (starting with \p Start) will be returned.<br>
>> > + LLVM_ATTRIBUTE_ALWAYS_INLINE<br>
>> > StringRef slice(size_t Start, size_t End) const {<br>
>> > Start = std::min(Start, Length);<br>
>> > End = std::min(std::max(Start, End), Length);<br>
>> > @@ -547,10 +565,12 @@ namespace llvm {<br>
>> > /// @name StringRef Comparison Operators<br>
>> > /// @{<br>
>> ><br>
>> > + LLVM_ATTRIBUTE_ALWAYS_INLINE<br>
>> > inline bool operator==(StringRef LHS, StringRef RHS) {<br>
>> > return LHS.equals(RHS);<br>
>> > }<br>
>> ><br>
>> > + LLVM_ATTRIBUTE_ALWAYS_INLINE<br>
>> > inline bool operator!=(StringRef LHS, StringRef RHS) {<br>
>> > return !(LHS == RHS);<br>
>> > }<br>
>> ><br>
>> ><br>
>> > _______________________________________________<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>