[PATCH] D153008: [RISCV] Allow slash-star comments in instruction operands

abel-bernabeu updated this revision to Diff 533194. abel-bernabeu added a comment. Even less verbosity... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153008/new/ https://reviews.llvm.org/D153008 Files:

[PATCH] D153008: [RISCV] Allow slash-star comments in instruction operands

abel-bernabeu updated this revision to Diff 533193. abel-bernabeu added a comment. Reduced the commit message verbosity by one half, without losing anything. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153008/new/

[PATCH] D153008: [RISCV] Allow slash-star comments in instruction operands

abel-bernabeu updated this revision to Diff 533191. abel-bernabeu added a comment. Addressed some review comments: - Doxygen-style comments are not common, so using triple-slash instead. - Using a regular C array on a place where the usage of SmallVector is not justified. - Proper

[PATCH] D153008: [RISCV] Allow slash-star comments in instruction operands

abel-bernabeu updated this revision to Diff 532837. abel-bernabeu added a comment. I just this flashback... For code coverage of one of the Lex() calls in ParseInstruction we needed one case where an instruction has no operands. We had, not one, but three. Am keeping just the "nop" case, which

[PATCH] D153008: [RISCV] Allow slash-star comments in instruction operands

abel-bernabeu added a comment. @jrtc27: The tests: - are under llvm/test/MC/RISCV - use llvm-mc as assembler (rather than C!) - are as simple as they can be - cover every line touched by this patch. Would you unblock this now? Thanks for your feedback so far. @MaskRay: Let me know if you

[PATCH] D153008: [RISCV] Allow slash-star comments in instruction operands

abel-bernabeu updated this revision to Diff 532758. abel-bernabeu added a comment. On the zdinx test, changed a FileCheck comment that was written starting with "//" rather than "#". Changed for "#" just for consistency with the rest of tests. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D153008: [RISCV] Allow slash-star comments in instruction operands

abel-bernabeu accepted this revision. abel-bernabeu added a comment. Am happy with the current patch version Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153008/new/ https://reviews.llvm.org/D153008

[PATCH] D153008: [RISCV] Allow slash-star comments in instruction operands

abel-bernabeu updated this revision to Diff 532754. abel-bernabeu added a comment. Removed a gratuitous blank line Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153008/new/ https://reviews.llvm.org/D153008 Files:

[PATCH] D153008: [RISCV] Allow slash-star comments in instruction operands

abel-bernabeu updated this revision to Diff 532753. abel-bernabeu added a comment. Added --match-fulllines to FileCheck for making the check stricter. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153008/new/ https://reviews.llvm.org/D153008

[PATCH] D153008: [RISCV] Allow slash-star comments in instruction operands

abel-bernabeu added inline comments. Comment at: llvm/test/MC/RISCV/comments.ll:25 +# CHECK: .Lpcrel_hi0:#c0 #c1 #c2 #c3 #c4 #c5 #c6 +# CHECK: auipc s0, %pcrel_hi(symbol+10) +# CHECK: addis0, s0, %pcrel_lo(.Lpcrel_hi0) This

[PATCH] D153008: [RISCV] Allow slash-star comments in instruction operands

abel-bernabeu updated this revision to Diff 532686. abel-bernabeu added a comment. Rebased on top of the latest changes in RISCVAsmParser.cpp Moved the testing to llc-mc test cases under lvm/test/MC/RISCV/ Covered with tests for every single case where the parser consumes a token and a

[PATCH] D153008: [RISCV] Allow slash-star comments in instruction operands

abel-bernabeu updated this revision to Diff 532489. abel-bernabeu added a comment. Fixed a typo on commit message Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153008/new/ https://reviews.llvm.org/D153008 Files:

[PATCH] D153008: [RISCV] Allow slash-star comments in instruction operands

abel-bernabeu updated this revision to Diff 532488. abel-bernabeu added a comment. Updated commit message: - fixed a typo - added a co-author Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153008/new/ https://reviews.llvm.org/D153008 Files:

[PATCH] D153008: [RISCV] Allow slash-star comments in instruction operands

abel-bernabeu added a comment. In D153008#4430491 , @MaskRay wrote: > In D153008#4428694 , @abel-bernabeu > wrote: > >> In D153008#4428568 , @MaskRay >> wrote: >> >>>

[PATCH] D153008: [RISCV] Allow slash-star comments in instruction operands

abel-bernabeu added a comment. In D153008#4424821 , @jrtc27 wrote: > Clang tests should not compile to asm. You want an IR test. My bad. You suggested IR as input (rather than C) and makes total sense. The latest update captures that suggestion.

[PATCH] D153008: [RISCV] Allow slash-star comments in instruction operands

abel-bernabeu updated this revision to Diff 532268. abel-bernabeu added a comment. The test has been moved to llvm/test/CodeGen/RISCV Also has been reworked for using LLVM IR as input, so clang is not needed in the loop. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D153008: [RISCV] Allow slash-star comments in instruction operands

abel-bernabeu added a comment. In D153008#4428568 , @MaskRay wrote: > This new update still applies many unneeded `getParser().Lex();` and adds a > test at a wrong layer >

[PATCH] D153008: [RISCV] Allow slash-star comments in instruction operands

abel-bernabeu marked an inline comment as done. abel-bernabeu added a comment. In D153008#4428568 , @MaskRay wrote: > This new update still applies many unneeded `getParser().Lex();` and adds a > test at a wrong layer >

[PATCH] D153008: [RISCV] Allow slash-star comments in instruction operands

abel-bernabeu added a comment. In D153008#4425244 , @jrtc27 wrote: > In D153008#4425238 , @abel-bernabeu > wrote: > >> In D153008#4424821 , @jrtc27 wrote: >> >>> Clang

[PATCH] D153008: [RISCV] Allow slash-star comments in instruction operands

abel-bernabeu updated this revision to Diff 532185. abel-bernabeu added a comment. Simplified the test. Also I check that the comments are a carried over to the assembly output. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153008/new/

[PATCH] D153150: [RISCV] Allow slash-star comments in instruction operands

abel-bernabeu updated this revision to Diff 532182. abel-bernabeu added a comment. Simplified the test and checking for comments to be carried over to the assembly output Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153150/new/

[PATCH] D153150: [RISCV] Allow slash-star comments in instruction operands

abel-bernabeu created this revision. Herald added subscribers: jobnoorman, luke, VincentWu, vkmr, frasercrmck, luismarques, apazos, sameer.abuasal, s.egerton, Jim, benna, psnobl, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, shiva0217, kito-cheng,

[PATCH] D153008: [RISCV] Allow slash-star comments in instruction operands

abel-bernabeu added a comment. Thanks everyone, taking notes of all the comments for improving the test: - Simplify the test (can be one instruction, no problem). - Check at IR level - Check for the comments to be placed in the output - Do "arc diff" with one-line descriptions Will update

[PATCH] D153008: [RISCV] Allow slash-star comments in instruction operands

abel-bernabeu added a comment. In D153008#4424821 , @jrtc27 wrote: > Clang tests should not compile to asm. You want an IR test. Jessica, are there any exceptions for tests is under CodeGen/RISCV intended to exercise the assembly parser? I have just

[PATCH] D153008: [RISCV] Allow slash-star comments in instruction operands

abel-bernabeu updated this revision to Diff 531746. abel-bernabeu added a comment. [RISCV] Allow slash-star comments in instruction operands It has been reported by one of Esperanto's customers that slash-start comments ("/*") within inline assembly were only allowed before the first instruction

[PATCH] D153008: [RISCV] Allow slash-star comments in instruction operands

abel-bernabeu updated this revision to Diff 531741. abel-bernabeu added a comment. [RISCV] Allow slash-star comments in instruction operands It has been reported by one of Esperanto's customers that slash-start comments ("/*") within inline assembly were only allowed before the first

[PATCH] D153008: [RISCV] Allow slash-star comments in instruction operands

abel-bernabeu added a comment. I was a bit reluctant to start a discussion on migrating getLexer().Lex() to getParser().Lex(), but I guess it makes sense to do it now rather than deferring. It is cleaner now, I agree. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D153008: [RISCV] Allow slash-star comments in instruction operands

abel-bernabeu updated this revision to Diff 531740. abel-bernabeu added a comment. [RISCV] Allow slash-star comments in instruction operands It has been reported by one of Esperanto's customers that slash-start comments ("/*") within inline assembly were only allowed before the first instruction

[PATCH] D153008: [RISCV] Allow slash-star comments in instruction operands

abel-bernabeu updated this revision to Diff 531735. abel-bernabeu added a comment. [RISCV] Allow slash-star comments in instruction operands It has been reported by one of Esperanto's customers that slash-start comments ("/*") within inline assembly were only allowed before the first

[PATCH] D153008: [RISCV] Allow slash-star comments in instruction operands

abel-bernabeu added a comment. For the customer who reported the problem, the comments are in the input source (doing their job explaining what the operands are). Now, if that comment is not seen when compiling with "-S" it is less of a problem that having the compilation not succeeding. I did

[PATCH] D153008: [RISCV] Allow slash-star comments in instruction operands

abel-bernabeu created this revision. Herald added subscribers: jobnoorman, luke, VincentWu, vkmr, frasercrmck, luismarques, apazos, sameer.abuasal, s.egerton, Jim, benna, psnobl, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, shiva0217, kito-cheng,