[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 @@
     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. 



More information about the llvm-commits mailing list