[PATCH] D99889: [AsmParser][ms][X86] Fix possible misbehaviour in parsing of special tokens at start of string.

Ricky Taylor via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Apr 6 13:54:41 PDT 2021


ricky26 added a comment.

In D99889#2669879 <https://reviews.llvm.org/D99889#2669879>, @anirudhp wrote:

>> It looks like this adds support for X86 identifiers starting with $ (I'm actually surprised this wasn't allowed before).
>
> Well. Yes and no. If you take a look at the `parseIdentifier` routine in AsmParser.cpp...

Right, that makes sense!

> An argument could be made that the parseIdentifier routine could be modified or overridden wherever appropriate and given custom logic.

The benefit being that target AsmParsers have the opportunity to parse the starting symbol as something else. The drawback being that stitching everything back together is more complicated (looking at the code, I'm not convinced that it'd parse $$ as an identifier, for example).

It doesn't look like this patch is going to affect other targets and it's simple.

I don't really have any other reservations. Probably worth getting a review from someone who knows the X86 backend better than me but otherwise LGTM.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99889/new/

https://reviews.llvm.org/D99889



More information about the llvm-commits mailing list