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, niosHD, sabuasal, simoncook, johnrusso, rbar, asb,
hiraditya, kristof.beyls, arichardson.
Herald added a project: All.
abel-bernabeu requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, pcwang-thead, eopXD,
MaskRay.
Herald added projects: clang, LLVM.
It had been reported by one of Esperanto's customers that slash-start
comments ("/*") within inline assembly were only allowed before the
first instruction operand or at the end of the lines. However, those
comments were not allowed when interleaved within the operands.
An example follows:
unsigned long int dst;
__asm__ __volatile__(
"li /* this is fine */ %[dst], /* this was NOT fine */ 0x1234\n"
"add zero, %[dst], %[dst]\n"
: [ dst ] "=r"(dst)
:
:);
A code review of the top level parser (AsmParser class) showed that
when comments are place before the instruction operand or at end of
a line, then they are gracefully handled irrespective of the backend.
When the comments are interleaved within the instruction operands it
is the backend's responsibility to handle the comments.
Explicitly handling the comments in the RISC-V backend is not a too
invasive patch and fixes the issue.
Thanks to David Spikett from Arm's community for pointing out where to
start looking within the LLVM code base.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D153008
Files:
clang/test/CodeGen/RISCV/riscv-inline-asm-gcc-commenting.c
llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
Index: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
===================================================================
--- llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
+++ llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
@@ -2573,14 +2573,26 @@
if (parseOperand(Operands, Name))
return true;
+ // Silently ignore comments after the first operand for compatibility with
gcc
+ while (getLexer().is(AsmToken::Comment))
+ getLexer().Lex();
+
// Parse until end of statement, consuming commas between operands
while (getLexer().is(AsmToken::Comma)) {
// Consume comma token
getLexer().Lex();
+ // Silently ignore comments before operand for compatibility with gcc
+ while (getLexer().is(AsmToken::Comment))
+ getLexer().Lex();
+
// Parse next operand
if (parseOperand(Operands, Name))
return true;
+
+ // Silently ignore comments after operand for compatibility with gcc
+ while (getLexer().is(AsmToken::Comment))
+ getLexer().Lex();
}
if (getParser().parseEOL("unexpected token")) {
Index: clang/test/CodeGen/RISCV/riscv-inline-asm-gcc-commenting.c
===================================================================
--- /dev/null
+++ clang/test/CodeGen/RISCV/riscv-inline-asm-gcc-commenting.c
@@ -0,0 +1,31 @@
+// RUN: %clang -fPIC --target=riscv64-unknown-elf -mabi=lp64f -O3 -o - -S %s |
FileCheck %s
+
+unsigned long int f1() {
+ unsigned long int dst;
+ __asm__ __volatile__("li %[dst], 0x1234 /* this is fine */ \n"
+ "add zero, %[dst], %[dst]\n"
+ : [ dst ] "=r"(dst)
+ :
+ :);
+ return dst;
+}
+
+unsigned long int f2() {
+ unsigned long int dst;
+ __asm__ __volatile__("li /* this is fine */ %[dst], /* this should also be
fine */ 0x1234\n"
+ "add zero, %[dst], %[dst]\n"
+ : [ dst ] "=r"(dst)
+ :
+ :);
+ return dst;
+}
+// CHECK: f1:
+// CHECK: lui a0, 1
+// CHECK-NEXT: addiw a0, a0, 564
+// CHECK-NEXT: add zero, a0, a0
+// CHECK: ret
+// CHECK: f2:
+// CHECK: lui a0, 1
+// CHECK-NEXT: addiw a0, a0, 564
+// CHECK-NEXT: add zero, a0, a0
+// CHECK: ret
Index: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
===================================================================
--- llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
+++ llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
@@ -2573,14 +2573,26 @@
if (parseOperand(Operands, Name))
return true;
+ // Silently ignore comments after the first operand for compatibility with gcc
+ while (getLexer().is(AsmToken::Comment))
+ getLexer().Lex();
+
// Parse until end of statement, consuming commas between operands
while (getLexer().is(AsmToken::Comma)) {
// Consume comma token
getLexer().Lex();
+ // Silently ignore comments before operand for compatibility with gcc
+ while (getLexer().is(AsmToken::Comment))
+ getLexer().Lex();
+
// Parse next operand
if (parseOperand(Operands, Name))
return true;
+
+ // Silently ignore comments after operand for compatibility with gcc
+ while (getLexer().is(AsmToken::Comment))
+ getLexer().Lex();
}
if (getParser().parseEOL("unexpected token")) {
Index: clang/test/CodeGen/RISCV/riscv-inline-asm-gcc-commenting.c
===================================================================
--- /dev/null
+++ clang/test/CodeGen/RISCV/riscv-inline-asm-gcc-commenting.c
@@ -0,0 +1,31 @@
+// RUN: %clang -fPIC --target=riscv64-unknown-elf -mabi=lp64f -O3 -o - -S %s | FileCheck %s
+
+unsigned long int f1() {
+ unsigned long int dst;
+ __asm__ __volatile__("li %[dst], 0x1234 /* this is fine */ \n"
+ "add zero, %[dst], %[dst]\n"
+ : [ dst ] "=r"(dst)
+ :
+ :);
+ return dst;
+}
+
+unsigned long int f2() {
+ unsigned long int dst;
+ __asm__ __volatile__("li /* this is fine */ %[dst], /* this should also be fine */ 0x1234\n"
+ "add zero, %[dst], %[dst]\n"
+ : [ dst ] "=r"(dst)
+ :
+ :);
+ return dst;
+}
+// CHECK: f1:
+// CHECK: lui a0, 1
+// CHECK-NEXT: addiw a0, a0, 564
+// CHECK-NEXT: add zero, a0, a0
+// CHECK: ret
+// CHECK: f2:
+// CHECK: lui a0, 1
+// CHECK-NEXT: addiw a0, a0, 564
+// CHECK-NEXT: add zero, a0, a0
+// CHECK: ret
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
