[llvm] r247253 - [ADT] Apply a large hammer to StringRef functions: attribute always_inline.

Pete Cooper via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 16 09:21:09 PDT 2015


> On Sep 16, 2015, at 9:16 AM, Benjamin Kramer <benny.kra at gmail.com> wrote:
> 
> 
>> On 16.09.2015, at 18:13, Pete Cooper via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>> 
>> 
>>> On Sep 15, 2015, at 9:36 PM, Chandler Carruth via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>>> 
>>> Sure, I hadn't realized how much this hurt debugging for you. How can we fix it?
>> How about adding attribute((used)) to these methods?
> 
> attribute((used)) will emit the function into every single translation unit that includes StringRef.h (aka everything in LLVM). That's something I'd like to avoid.
Of course.  I forgot about that.  I thought that ‘used’ would basically get us back to the behavior we had before, but thats not true when the method is defined in the class.

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



More information about the llvm-commits mailing list