[lld] r228077 - Simplify large switches.

Chandler Carruth chandlerc at google.com
Tue Feb 3 22:29:46 PST 2015


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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150203/48501ecc/attachment.html>


More information about the llvm-commits mailing list