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 instruction operand or at the end of the lines. Those comments
  were, however, not allowed when interleaved within the operands.
  
  An example follows:
  
  unsigned long int dst;
  __asm__ __volatile__(
    "li /* this was 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 were placed before the instruction operand or at end of
  a line, then they were gracefully handled irrespective of the backend.
  When the comments were interleaved within the instruction operands it
  was the backend's responsibility to handle the comments.
  
  RISC-V's backend did not handle the comments in any way.
  
  Beyond the obvious solution of explicitly handling the comments within
  the RISC-V backend, another, easier to maintain solution was suggested
  by Sergei Barannikov in a Discourse discussion thread:
  
  
https://discourse.llvm.org/t/interleaving-several-c-style-comments-in-the-same-inline-assembly-line/71353/8
  
  In summary, all backends, including the RISC-V's, should switch
  from getLexer().Lex() to getParser().Lex() in their ParseInstruction
  implementation.
  
  The getLexer().Lex() approach relies on the user to explicitly handle
  the comments, whereas the suggested getParser().Lex() alternive already
  handles the comments in the same way as done for non-target-specific
  assembly directives.
  
  Here we just do the RISC-V work. Other backends should also do their own
  review.
  
  In addition to Sergei Barannikov, I would also we thank David Spikett
  from Arm's community for pointing out where to start looking within the
  LLVM code base, and also the patch reviewers.


Repository:
  rG LLVM Github Monorepo

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

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
@@ -1617,7 +1617,7 @@
       Operands.push_back(RISCVOperand::createToken("(", FirstS));
     SMLoc S = getLoc();
     SMLoc E = SMLoc::getFromPointer(S.getPointer() + Name.size());
-    getLexer().Lex();
+    getParser().Lex();
     Operands.push_back(RISCVOperand::createReg(RegNo, S, E));
   }
 
@@ -1978,11 +1978,11 @@
     return MatchOperand_Success;
   case AsmToken::Plus:
     Opcode = MCBinaryExpr::Add;
-    getLexer().Lex();
+    getParser().Lex();
     break;
   case AsmToken::Minus:
     Opcode = MCBinaryExpr::Sub;
-    getLexer().Lex();
+    getParser().Lex();
     break;
   }
 
@@ -2131,11 +2131,11 @@
                       MaskAgnostic))
     return MatchOperand_NoMatch;
 
-  getLexer().Lex();
+  getParser().Lex();
 
   while (getLexer().is(AsmToken::Comma)) {
     // Consume comma.
-    getLexer().Lex();
+    getParser().Lex();
 
     if (getLexer().isNot(AsmToken::Identifier))
       break;
@@ -2146,7 +2146,7 @@
                         MaskAgnostic))
       break;
 
-    getLexer().Lex();
+    getParser().Lex();
   }
 
   if (getLexer().is(AsmToken::EndOfStatement) && State == VTypeState_Done) {
@@ -2186,7 +2186,7 @@
     return MatchOperand_NoMatch;
   SMLoc S = getLoc();
   SMLoc E = SMLoc::getFromPointer(S.getPointer() + Name.size());
-  getLexer().Lex();
+  getParser().Lex();
   Operands.push_back(RISCVOperand::createReg(RegNo, S, E));
   return MatchOperand_Success;
 }
@@ -2202,7 +2202,7 @@
     return MatchOperand_NoMatch;
   SMLoc S = getLoc();
   SMLoc E = SMLoc::getFromPointer(S.getPointer() + Name.size());
-  getLexer().Lex();
+  getParser().Lex();
   Operands.push_back(RISCVOperand::createReg(
       RegNo, S, E, !getSTI().hasFeature(RISCV::FeatureStdExtF)));
   return MatchOperand_Success;
@@ -2391,11 +2391,11 @@
     Error(getLoc(), "register list must start from 'ra' or 'x1'");
     return MatchOperand_ParseFail;
   }
-  getLexer().Lex();
+  getParser().Lex();
 
   // parse case like ,s0
   if (getLexer().is(AsmToken::Comma)) {
-    getLexer().Lex();
+    getParser().Lex();
     if (getLexer().isNot(AsmToken::Identifier)) {
       Error(getLoc(), "invalid register");
       return MatchOperand_ParseFail;
@@ -2410,12 +2410,12 @@
       Error(getLoc(), "continuous register list must start from 's0' or 'x8'");
       return MatchOperand_ParseFail;
     }
-    getLexer().Lex(); // eat reg
+    getParser().Lex(); // eat reg
   }
 
   // parse case like -s1
   if (getLexer().is(AsmToken::Minus)) {
-    getLexer().Lex();
+    getParser().Lex();
     StringRef EndName = getLexer().getTok().getIdentifier();
     // FIXME: the register mapping and checks of EABI is wrong
     RegEnd = matchRegisterNameHelper(IsEABI, EndName);
@@ -2428,7 +2428,7 @@
                       "'x8-x9' pair");
       return MatchOperand_ParseFail;
     }
-    getLexer().Lex();
+    getParser().Lex();
   }
 
   if (!IsEABI) {
@@ -2442,7 +2442,7 @@
       }
 
       // parse ', x18' for extra part
-      getLexer().Lex();
+      getParser().Lex();
       if (getLexer().isNot(AsmToken::Identifier)) {
         Error(getLoc(), "invalid register");
         return MatchOperand_ParseFail;
@@ -2453,11 +2453,11 @@
                         "must start from 'x18'");
         return MatchOperand_ParseFail;
       }
-      getLexer().Lex();
+      getParser().Lex();
 
       // parse '-x20' for extra part
       if (getLexer().is(AsmToken::Minus)) {
-        getLexer().Lex();
+        getParser().Lex();
         if (getLexer().isNot(AsmToken::Identifier)) {
           Error(getLoc(), "invalid register");
           return MatchOperand_ParseFail;
@@ -2467,7 +2467,7 @@
           Error(getLoc(), "invalid register");
           return MatchOperand_ParseFail;
         }
-        getLexer().Lex();
+        getParser().Lex();
       }
       RegEnd = MatchRegisterName(EndName);
     }
@@ -2497,7 +2497,7 @@
 
 OperandMatchResultTy RISCVAsmParser::parseZcmpSpimm(OperandVector &Operands) {
   if (getLexer().is(AsmToken::Minus))
-    getLexer().Lex();
+    getParser().Lex();
 
   SMLoc S = getLoc();
   int64_t StackAdjustment = getLexer().getTok().getIntVal();
@@ -2508,7 +2508,7 @@
   if (!RISCVZC::getSpimm(RlistVal, Spimm, StackAdjustment, isRV64(), IsEABI))
     return MatchOperand_NoMatch;
   Operands.push_back(RISCVOperand::createSpimm(Spimm << 4, S));
-  getLexer().Lex();
+  getParser().Lex();
   return MatchOperand_Success;
 }
 
@@ -2576,7 +2576,7 @@
   // Parse until end of statement, consuming commas between operands
   while (getLexer().is(AsmToken::Comma)) {
     // Consume comma token
-    getLexer().Lex();
+    getParser().Lex();
 
     // Parse next operand
     if (parseOperand(Operands, Name))
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

Reply via email to