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

Chandler Carruth via llvm-commits llvm-commits at lists.llvm.org
Mon Sep 21 10:42:15 PDT 2015


This is my plan. About to commit. Please let me know if it helps.

On Wed, Sep 16, 2015 at 12:53 PM Justin Bogner <mail at justinbogner.com>
wrote:

> Chandler Carruth <chandlerc at gmail.com> writes:
> > Sure, I hadn't realized how much this hurt debugging for you. How can we
> > fix it? 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.
>
> The case that's been coming up for me is basically just the "char *"
> constructor - calling functions with a "char *" and getting the
> conversion is pretty common while debugging. It seems like a reasonable
> trade off to leave the attribute on the non-constructor methods, which
> mostly just access attributes you can get at in the debugger anyway, and
> just drop it for the constructors. WDYT?
>
> > 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
> >>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150921/043c6623/attachment.html>


More information about the llvm-commits mailing list