[PATCH] D17293: [MC] AsmLexer: 30% speedup on tests, added extensible identifier's character set support.

Valery Pykhtin via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 1 11:34:09 PST 2016


vpykhtin added inline comments.

================
Comment at: include/llvm/MC/MCParser/MCAsmLexer.h:211
@@ +210,3 @@
+  /// allow/disallow an identifier to contain specified characters
+  virtual void setIdentifierCharSet(bool Value,
+                                    StringRef PfxCharSet,
----------------
rafael wrote:
> Why do you need to make these virtual?
Well not making it virtual would require bitvector sets to be part of this class. I'm not objecting though as it already done with SkipSpace and AllowAtInIdentifier.

================
Comment at: include/llvm/MC/MCParser/MCAsmLexer.h:216
@@ +215,3 @@
+  /// test whether the specified character can be found in an identifier
+  virtual bool isIdentifierCharSetContains(char) const = 0;
+
----------------
rafael wrote:
> is..Contains is a strange name since it has two verbs.
What would be a better name here?

================
Comment at: lib/MC/MCParser/AsmLexer.cpp:23
@@ -23,1 +22,3 @@
 
+// standard is(x)digit generally much slower than simple
+// checks like below
----------------
rafael wrote:
> Why?
Well it based on my previuos experience on Windows where we had lexer using these routines eating up to 10% of scan time. Probably not so "generally" as I stated though. I'm not insisting on this particular change and can remove it.


Repository:
  rL LLVM

http://reviews.llvm.org/D17293





More information about the llvm-commits mailing list