[PATCH] D142865: [RISCV] Use custom operand parsing for FenceArg.

Mon Jan 30 08:40:59 PST 2023

craig.topper added inline comments.

================
Comment at: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:1854-1865
+    case 'i':
+      Imm |= RISCVFenceField::I;
+      break;
+    case 'o':
+      Imm |= RISCVFenceField::O;
+      break;
+    case 'r':
----------------
luismarques wrote:
> frasercrmck wrote:
> > luismarques wrote:
> > > In a follow-up patch, we should reject repeated letters, like `iiorrrrrw`. Binutils seems to reject that, which IMO is the right decision.
> > I took the `c <= Prev` check below to do just that
> Oops, for some reason I didn't notice that. Thanks, @frasercrmck.
> The tests should be extended to cover this tighter checking, though.
> The patch summary should also mention that change. I'm not sure if that is noteworthy enough for release notes? It is a user-visible change...
That code isn’t new. It was already in isFenceArg in the old code.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142865