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

[Rafael Espíndola via llvm-commits]

On 1 March 2016 at 14:34, Valery Pykhtin <valery.pykhtin at amd.com> wrote:
> 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.

Yes, please make it non virtual.

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

Just isAllowedChar?


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

Yes, please leave it out. It can be done as an independent patch.

BTW, how exactly have you benchmarked this?

Cheers,
Rafael