[PATCH] D20337: [MC] Support symbolic expressions in assembly directives

Nirav Dave via llvm-commits llvm-commits at lists.llvm.org
Mon May 23 19:54:09 PDT 2016


niravd added inline comments.

================
Comment at: include/llvm/MC/MCStreamer.h:591
@@ +590,3 @@
+  /// \param Loc - The location of the expression for error reporting.
+  virtual void emitSpace(const MCExpr *Size, uint64_t FillValue,
+                         SMLoc Loc = SMLoc());
----------------
Please rename to match casing of other Emit functions. (EmitSpace). 

================
Comment at: include/llvm/MC/MCStreamer.h:602
@@ +601,3 @@
+  /// \param Loc - The location of the expression for error reporting.
+  virtual void emitRepeatedValue(const MCExpr *NumValues, int64_t Size,
+                                 int64_t Expr, SMLoc Loc = SMLoc());
----------------
Why isn't this named EmitFill? It's of a distinct type from the other calls and is only for dealing with Fill directives and the distinction seems more confusing than multiple variants of EmitFill. 

================
Comment at: include/llvm/MC/MCStreamer.h:605
@@ +604,3 @@
+
+  void emitRepeatedValueImpl(int64_t Repeat, int64_t Size, int64_t Value,
+                             SMLoc Loc = SMLoc());
----------------
Save as above.

================
Comment at: lib/MC/MCAsmStreamer.cpp:828
@@ +827,3 @@
+
+  OS << "\t.space\t";
+  NumBytes->print(OS, MAI);
----------------
Since we cannot guarantee a lack of errors at this point we should emit line directives before and after this so that we preserve the error message location with respect to the original file. 

================
Comment at: lib/MC/MCParser/AsmParser.cpp:2749
@@ -2747,2 +2748,3 @@
     Lex();
     if (parseAbsoluteExpression(Val))
+      return false;
----------------
Please decompose this into parseExpression and evaluateAsAbsolute to distinguish expression structure errors (should throw an error here) from value errors which we may defer. 


Repository:
  rL LLVM

http://reviews.llvm.org/D20337





More information about the llvm-commits mailing list