[lld] r228077 - Simplify large switches.

Rui Ueyama ruiu at google.com
Wed Feb 4 14:02:56 PST 2015


Comment is back in r228201.

As to the performance, LLD took 200 *microseconds* in
GnuLdDriver::evaluateLinkerScript when linking itself on Linux (it parsed
600 bytes of data). And these functions are only a small part of the entire
process. It's really negligible.

On Tue, Feb 3, 2015 at 10:29 PM, Chandler Carruth <chandlerc at google.com>
wrote:

> Comments are great, but I do much prefer the 'strchr' interface. So lets
> go with comments and a strchr interface.
>
> If this doesn't generate good code for some reason (no idea, haven't
> checked) we could either teach LLVM to generate awesome code for it or we
> could provide our own strchr-like utility that generates awesome code....
> But i'd like to see evidence that the performance of these is critical.
>
> On Tue, Feb 3, 2015 at 9:21 PM, Philip Reames <listmail at philipreames.com>
> wrote:
>
>> To me the first change is a readability regression (lost comments if
>> nothing else); the later two are neutral.  I don't have a strong
>> preference, but please preserve the comments from the first function if
>> nothing else.
>>
>> p.s. Has anyone checked if we convert these strchrs to the "obvious" bits
>> of code?  Each of these should be nearly as fast as the original switches
>> provided they're optimized correctly.
>>
>>
>> On 02/03/2015 06:33 PM, Shankar Easwaran wrote:
>>
>>> The style used in the old code is followed in clang too in various
>>> places. I felt its more readable too.
>>>
>>> This makes it less performant even if its not in performance critical
>>> pass and less readable too.
>>>
>>> Shankar Easwaran
>>>
>>> On 2/3/2015 6:00 PM, Rui Ueyama wrote:
>>>
>>>> Author: ruiu
>>>> Date: Tue Feb  3 18:00:21 2015
>>>> New Revision: 228077
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=228077&view=rev
>>>> Log:
>>>> Simplify large switches.
>>>>
>>>> This may be a little bit inefficient than the original code
>>>> but that should be okay as this is not really in a performance
>>>> critical pass.
>>>>
>>>> http://reviews.llvm.org/D7393
>>>>
>>>> Modified:
>>>>      lld/trunk/lib/ReaderWriter/LinkerScript.cpp
>>>>
>>>> Modified: lld/trunk/lib/ReaderWriter/LinkerScript.cpp
>>>> URL: http://llvm.org/viewvc/llvm-project/lld/trunk/lib/
>>>> ReaderWriter/LinkerScript.cpp?rev=228077&r1=228076&r2=228077&view=diff
>>>> ==============================================================================
>>>>
>>>> --- lld/trunk/lib/ReaderWriter/LinkerScript.cpp (original)
>>>> +++ lld/trunk/lib/ReaderWriter/LinkerScript.cpp Tue Feb  3 18:00:21
>>>> 2015
>>>> @@ -242,66 +242,17 @@ bool Lexer::canStartNumber(char c) const
>>>>   }
>>>>     bool Lexer::canContinueNumber(char c) const {
>>>> -  switch (c) {
>>>> -  // Digits
>>>> -  case '0': case '1': case '2': case '3': case '4': case '5': case '6':
>>>> -  case '7': case '8': case '9':
>>>> -  case 'A': case 'B': case 'C': case 'D': case 'E': case 'F':
>>>> -  case 'a': case 'b': case 'c': case 'd': case 'e': case 'f':
>>>> -  // Hex marker
>>>> -  case 'x': case 'X':
>>>> -  // Type suffix
>>>> -  case 'h': case 'H': case 'o': case 'O':
>>>> -  // Scale suffix
>>>> -  case 'M': case 'K':
>>>> -    return true;
>>>> -  default:
>>>> -    return false;
>>>> -  }
>>>> +  return strchr("0123456789ABCDEFabcdefxXhHoOMK", c);
>>>>   }
>>>>     bool Lexer::canStartName(char c) const {
>>>> -  switch (c) {
>>>> -  case 'A': case 'B': case 'C': case 'D': case 'E': case 'F': case 'G':
>>>> -  case 'H': case 'I': case 'J': case 'K': case 'L': case 'M': case 'N':
>>>> -  case 'O': case 'P': case 'Q': case 'R': case 'S': case 'T': case 'U':
>>>> -  case 'V': case 'W': case 'X': case 'Y': case 'Z':
>>>> -  case 'a': case 'b': case 'c': case 'd': case 'e': case 'f': case 'g':
>>>> -  case 'h': case 'i': case 'j': case 'k': case 'l': case 'm': case 'n':
>>>> -  case 'o': case 'p': case 'q': case 'r': case 's': case 't': case 'u':
>>>> -  case 'v': case 'w': case 'x': case 'y': case 'z':
>>>> -  case '_': case '.': case '$': case '/': case '\\':
>>>> -  case '*':
>>>> -    return true;
>>>> -  default:
>>>> -    return false;
>>>> -  }
>>>> +  return strchr(
>>>> + "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz_.$/\\*", c);
>>>>   }
>>>>     bool Lexer::canContinueName(char c) const {
>>>> -  switch (c) {
>>>> -  case 'A': case 'B': case 'C': case 'D': case 'E': case 'F': case 'G':
>>>> -  case 'H': case 'I': case 'J': case 'K': case 'L': case 'M': case 'N':
>>>> -  case 'O': case 'P': case 'Q': case 'R': case 'S': case 'T': case 'U':
>>>> -  case 'V': case 'W': case 'X': case 'Y': case 'Z':
>>>> -  case 'a': case 'b': case 'c': case 'd': case 'e': case 'f': case 'g':
>>>> -  case 'h': case 'i': case 'j': case 'k': case 'l': case 'm': case 'n':
>>>> -  case 'o': case 'p': case 'q': case 'r': case 's': case 't': case 'u':
>>>> -  case 'v': case 'w': case 'x': case 'y': case 'z':
>>>> -  case '0': case '1': case '2': case '3': case '4': case '5': case '6':
>>>> -  case '7': case '8': case '9':
>>>> -  case '_': case '.': case '$': case '/': case '\\': case '~': case
>>>> '=':
>>>> -  case '+':
>>>> -  case '[':
>>>> -  case ']':
>>>> -  case '*':
>>>> -  case '?':
>>>> -  case '-':
>>>> -  case ':':
>>>> -    return true;
>>>> -  default:
>>>> -    return false;
>>>> -  }
>>>> +  return strchr("ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"
>>>> +                "0123456789_.$/\\~=+[]*?-:", c);
>>>>   }
>>>>     /// Helper function to split a StringRef in two at the nth
>>>> character.
>>>>
>>>>
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>>
>>>>
>>>>
>>>
>>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150204/604d803f/attachment.html>


More information about the llvm-commits mailing list