[PATCH] D138352: [RISCV] Support .variant_cc directive for the assembler.

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:2313
+  if (!Sym)
+    return TokError("unknown symbol in '.variant_cc' directive");
+
----------------
` in '.variant_cc' directive` is not really useful as the diagnostic repeats the source line. ditto below


================
Comment at: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:2317
+
+  // There shouldn't be any more tokens.
+  if (!Parser.parseEOL("unexpected token in '.variant_cc' directive"))
----------------
This code self explains so the comment seems redundant. You may delete the blank line after `Parser.Lex()` as well.


================
Comment at: llvm/test/MC/RISCV/directive-variant_cc-err.s:4
+.variant_cc
+// CHECK: error: expected symbol name
+// CHECK-NEXT:   .variant_cc
----------------
Add line/column information like `:[[#@LINE+1]]:25:`. See some newer tests in llvm/test/MC/

Grep `defsym=ERR=1`. It's probably better to combine positive and negative tests in one file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138352