[PATCH] D153204: RISCVAsmParser: support comments in more places

Tue Jun 20 22:05:22 PDT 2023

MaskRay added a comment.

In D153204#4430646 <https://reviews.llvm.org/D153204#4430646>, @abel-bernabeu wrote:

> 1. Use "LI" rather than "VSETIVLI". It really does not matter what instruction you use for testing the handling of comments. Think of those backporting the patch to very old versions of LLVM.

Answered. The special parsing of VSETIVLI needs a test, not LI.

> 2. Make an independent file for this test. When the test fails because of the handling of comments the test name would point very straight to the issue.

Not sure about this. See https://maskray.me/blog/2021-08-08-toolchain-testing "I don't know an existing test can be enhanced"

> 3. Check that the comments are placed into the assembler rather than ignored.

Not sure about this. See my blog post about "The test checks at the wrong layer".

How to preserve comments can be tested elsewhere, e.g. `test/MC/AsmParser/preserve-comments*`, `test/MC/AsmParser/inline-comments.ll`.
These tests may need improvement, but that is a separate issue.

> We spent a bit of time on the review of the original patch proposal for figuring out those things already. Cherry-pick my test at least :-D
>
> https://reviews.llvm.org/D153008

If the important thing is to not miss "It has been reported by one of Esperanto's customers". I can mention this as well.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153204