[lld] r228077 - Simplify large switches.

Philip Reames listmail at philipreames.com
Tue Feb 3 21:21:02 PST 2015


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
>>
>>
>
>




More information about the llvm-commits mailing list