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

Mon Jan 30 04:08:25 PST 2023

luismarques 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':
----------------
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...

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142865