<div dir="ltr">Comments are great, but I do much prefer the 'strchr' interface. So lets go with comments and a strchr interface.<div><br></div><div>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.</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Feb 3, 2015 at 9:21 PM, Philip Reames <span dir="ltr"><<a href="mailto:listmail@philipreames.com" target="_blank">listmail@philipreames.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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.<br>
<br>
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.<div class="HOEnZb"><div class="h5"><br>
<br>
On 02/03/2015 06:33 PM, Shankar Easwaran wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
The style used in the old code is followed in clang too in various places. I felt its more readable too.<br>
<br>
This makes it less performant even if its not in performance critical pass and less readable too.<br>
<br>
Shankar Easwaran<br>
<br>
On 2/3/2015 6:00 PM, Rui Ueyama wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Author: ruiu<br>
Date: Tue Feb  3 18:00:21 2015<br>
New Revision: 228077<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=228077&view=rev" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project?rev=228077&view=rev</a><br>
Log:<br>
Simplify large switches.<br>
<br>
This may be a little bit inefficient than the original code<br>
but that should be okay as this is not really in a performance<br>
critical pass.<br>
<br>
<a href="http://reviews.llvm.org/D7393" target="_blank">http://reviews.llvm.org/D7393</a><br>
<br>
Modified:<br>
     lld/trunk/lib/ReaderWriter/<u></u>LinkerScript.cpp<br>
<br>
Modified: lld/trunk/lib/ReaderWriter/<u></u>LinkerScript.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/LinkerScript.cpp?rev=228077&r1=228076&r2=228077&view=diff" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/lld/trunk/lib/<u></u>ReaderWriter/LinkerScript.cpp?<u></u>rev=228077&r1=228076&r2=<u></u>228077&view=diff</a><br>
==============================<u></u>==============================<u></u>================== <br>
--- lld/trunk/lib/ReaderWriter/<u></u>LinkerScript.cpp (original)<br>
+++ lld/trunk/lib/ReaderWriter/<u></u>LinkerScript.cpp Tue Feb  3 18:00:21 2015<br>
@@ -242,66 +242,17 @@ bool Lexer::canStartNumber(char c) const<br>
  }<br>
    bool Lexer::canContinueNumber(char c) const {<br>
-  switch (c) {<br>
-  // Digits<br>
-  case '0': case '1': case '2': case '3': case '4': case '5': case '6':<br>
-  case '7': case '8': case '9':<br>
-  case 'A': case 'B': case 'C': case 'D': case 'E': case 'F':<br>
-  case 'a': case 'b': case 'c': case 'd': case 'e': case 'f':<br>
-  // Hex marker<br>
-  case 'x': case 'X':<br>
-  // Type suffix<br>
-  case 'h': case 'H': case 'o': case 'O':<br>
-  // Scale suffix<br>
-  case 'M': case 'K':<br>
-    return true;<br>
-  default:<br>
-    return false;<br>
-  }<br>
+  return strchr("<u></u>0123456789ABCDEFabcdefxXhHoOMK<u></u>", c);<br>
  }<br>
    bool Lexer::canStartName(char c) const {<br>
-  switch (c) {<br>
-  case 'A': case 'B': case 'C': case 'D': case 'E': case 'F': case 'G':<br>
-  case 'H': case 'I': case 'J': case 'K': case 'L': case 'M': case 'N':<br>
-  case 'O': case 'P': case 'Q': case 'R': case 'S': case 'T': case 'U':<br>
-  case 'V': case 'W': case 'X': case 'Y': case 'Z':<br>
-  case 'a': case 'b': case 'c': case 'd': case 'e': case 'f': case 'g':<br>
-  case 'h': case 'i': case 'j': case 'k': case 'l': case 'm': case 'n':<br>
-  case 'o': case 'p': case 'q': case 'r': case 's': case 't': case 'u':<br>
-  case 'v': case 'w': case 'x': case 'y': case 'z':<br>
-  case '_': case '.': case '$': case '/': case '\\':<br>
-  case '*':<br>
-    return true;<br>
-  default:<br>
-    return false;<br>
-  }<br>
+  return strchr(<br>
+ "<u></u>ABCDEFGHIJKLMNOPQRSTUVWXYZabcd<u></u>efghijklmnopqrstuvwxyz_.$/\\*"<u></u>, c);<br>
  }<br>
    bool Lexer::canContinueName(char c) const {<br>
-  switch (c) {<br>
-  case 'A': case 'B': case 'C': case 'D': case 'E': case 'F': case 'G':<br>
-  case 'H': case 'I': case 'J': case 'K': case 'L': case 'M': case 'N':<br>
-  case 'O': case 'P': case 'Q': case 'R': case 'S': case 'T': case 'U':<br>
-  case 'V': case 'W': case 'X': case 'Y': case 'Z':<br>
-  case 'a': case 'b': case 'c': case 'd': case 'e': case 'f': case 'g':<br>
-  case 'h': case 'i': case 'j': case 'k': case 'l': case 'm': case 'n':<br>
-  case 'o': case 'p': case 'q': case 'r': case 's': case 't': case 'u':<br>
-  case 'v': case 'w': case 'x': case 'y': case 'z':<br>
-  case '0': case '1': case '2': case '3': case '4': case '5': case '6':<br>
-  case '7': case '8': case '9':<br>
-  case '_': case '.': case '$': case '/': case '\\': case '~': case '=':<br>
-  case '+':<br>
-  case '[':<br>
-  case ']':<br>
-  case '*':<br>
-  case '?':<br>
-  case '-':<br>
-  case ':':<br>
-    return true;<br>
-  default:<br>
-    return false;<br>
-  }<br>
+  return strchr("<u></u>ABCDEFGHIJKLMNOPQRSTUVWXYZabcd<u></u>efghijklmnopqrstuvwxyz"<br>
+                "0123456789_.$/\\~=+[]*?-:", c);<br>
  }<br>
    /// Helper function to split a StringRef in two at the nth character.<br>
<br>
<br>
______________________________<u></u>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/<u></u>mailman/listinfo/llvm-commits</a><br>
<br>
<br>
</blockquote>
<br>
<br>
</blockquote>
<br>
______________________________<u></u>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/<u></u>mailman/listinfo/llvm-commits</a><br>
</div></div></blockquote></div><br></div>