[PATCH] D91735: [PowerPC] Allow a '%' prefix for registers in CFI directives

[Nick Desaulniers via Phabricator via llvm-commits]

nickdesaulniers added a comment.

I was thinking "would it be better to move this logic down into `PPCAsmParser::MatchRegisterName`? Well, I would suppose that would depend on if the `%` prefix on registers is valid in other contexts, for instance maybe other places that directly call `PPCAsmParser::MatchRegisterName` rather than ``PPCAsmParser::tryParseRegister` which you've modified here.

Right away, the next caller of `PPCAsmParser::MatchRegisterName` I found is in `PPCAsmParser::ParseOperand`:

  1480   switch (getLexer().getKind()) {                                                                                                                                                     
  1481   // Special handling for register names.  These are interpreted                                                                                                                      
  1482   // as immediates corresponding to the register number.                                                                                                                              
  1483   case AsmToken::Percent:                                                                                                                                                             
  1484     Parser.Lex(); // Eat the '%'.                                                                                                                                                     
  1485     unsigned RegNo;                                                                                                                                                                   
  1486     int64_t IntVal;                                                                                                                                                                   
  1487     if (MatchRegisterName(RegNo, IntVal))                                                                                                                                             
  1488       return Error(S, "invalid register name");
  ...
  1553     switch (getLexer().getKind()) {                                                                                                                                                   
  1554     case AsmToken::Percent:                                                                                                                                                           
  1555       Parser.Lex(); // Eat the '%'.                                                                                                                                                   
  1556       unsigned RegNo;                                                                                                                                                                 
  1557       if (MatchRegisterName(RegNo, IntVal)) 

So it looks like it's currently idiosyncratic that some callers lex the `%` and others do not.  I highly doubt that's intentional.  In that case, would you mind please sinking the lexing of `%` into `PPCAsmParser::MatchRegisterName`, then removing the existing `%` lexing, that way all sites that expect to parse registers conditionally with `%` prefixed do so in one place?  I guess the existing sites expect there to be a `%` unconditionally; I suspect it should be conditional, at least to match GNU `as` portability concerns.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91735