[PATCH] D123515: [RISCV] Support '.option arch' directive

[Jessica Clarke via Phabricator via llvm-commits]

jrtc27 added inline comments.


================
Comment at: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:2135
+
+    if (Parser.parseOptionalToken(AsmToken::Equal)) {
+      SMLoc ArchLoc = Parser.getTok().getLoc();
----------------
binutils dropped the leading = but the spec proposal still has it


================
Comment at: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:2200
+    Parser.Lex();
+    return Parser.parseToken(AsmToken::EndOfStatement,
+                             "unexpected token, expected end of statement");
----------------
binutils allows more than one change in the same line as `.option arch, +foo, bar, -baz, +qwerty` etc


================
Comment at: llvm/test/MC/RISCV/option-invalid.s:32
+.option arch, +d
+# CHECK: error: Can't disable f extension, d extension requires f extension be enabled
+.option arch, -f
----------------
Allowing this would be more useful IMO, otherwise you need to know the full list of extensions you might need to disable in order to disable another extension, which can vary with the toolchain.

However, binutils takes an alternative approach at the moment and re-computes the implied extensions at the end, so this becomes a no-op as rv32id gets turned back into rv32ifd. I don't think that's particularly helpful behaviour though, I would even call it surprising.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123515