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

Rafael Ávila de Espíndola via llvm-commits llvm-commits at lists.llvm.org
Wed May 25 06:32:28 PDT 2016


rafael accepted this revision.
rafael added a reviewer: rafael.
rafael added a comment.
This revision is now accepted and ready to land.

LGTM with nits.

I think the interface can be simplified further, but that should be a follow up. What I think should be done in another patch is to make the versions that take an integer be non virtual and just create and forward to the version that takes an expression.


================
Comment at: include/llvm/MC/MCObjectStreamer.h:144
@@ -143,1 +143,3 @@
   void EmitFill(uint64_t NumBytes, uint8_t FillValue) override;
+  void emitFill(const MCExpr *NumBytes, uint64_t FillValue,
+                SMLoc Loc = SMLoc()) override;
----------------
NumBytes can be a reference, no?

================
Comment at: include/llvm/MC/MCStreamer.h:584
@@ +583,3 @@
+  ///
+  /// \param Size - The size of space (in bytes) to emit.
+  /// \param Value - The value to use when filling bytes.
----------------
Size is not a parameter of this version, nor is Value.

================
Comment at: include/llvm/MC/MCStreamer.h:595
@@ +594,3 @@
+  ///
+  /// \param Size - The size of space (in bytes) to emit.
+  /// \param Value - The value to use when filling bytes.
----------------
This don't match the signature.

================
Comment at: lib/MC/MCAsmStreamer.cpp:828
@@ +827,3 @@
+  MCStreamer::emitFill(NumBytes, FillValue);
+}
+
----------------
niravd wrote:
> I believe it still is. Consider the case where the symbolic expression emitted for NumBytes is eventually evaluated to a non-positive number or is invalid? Errors in the compilation of the output assembly would point to location in the output asm file not the original as it should. 
> 
> 
This is the asm streamer, so it will just print a negative number in the .s.

================
Comment at: lib/MC/MCAsmStreamer.cpp:831
@@ +830,3 @@
+void MCAsmStreamer::emitFill(uint64_t NumValues, int64_t Size, int64_t Expr) {
+  if (NumValues == 0) return;
+
----------------
git-clang-format the patch


Repository:
  rL LLVM

http://reviews.llvm.org/D20337





More information about the llvm-commits mailing list