[PATCH] Fix PR18345: ldr= pseudo instruction produces incorrect code when using in inline assembly

[Rafael Ávila de Espíndola]

  It seems strange that the asm streamer doesn't simply print "ldr ..." and ".ltorg". Why is that? It should be just a case of implementing those methods in the assembly streamer. With it they would just pass through a "llvm-mc -filetype=asm".

  Another advantage is that with the asm streamer just printing the directives, we could move

  OwningPtr<AssemblerConstantPools> ConstantPools;

  into a ARMTargetObjectStreamer.

  Instead of having do nothing methods in the macho streamer, please move them to the arm streamer (or ARMTargetObjectStreamer if you create one) and have them just call llvm_unreachable.


================
Comment at: include/llvm/MC/MCStreamer.h:85
@@ -80,3 +84,3 @@
 // FIXME: declared here because it is used from
 // lib/CodeGen/AsmPrinter/ARMException.cpp.
 class ARMTargetStreamer : public MCTargetStreamer {
----------------
I wonder if we should keep only the methods used by ARMException.cpp in here. Or maybe the debug info emission needs its own virtual interface.

Just thinking to myself, it we do that it should be in an another patch :-)


================
Comment at: include/llvm/MC/MCStreamer.h:116
@@ +115,3 @@
+  // Callbacks to handle assembler-generated constant pools
+  /// finish() - write out any non-empty assembler constant pools.
+  virtual void finish();
----------------
That is what the implementation does, not what the interface is for. I would move this comment to the implementation.

================
Comment at: include/llvm/MC/MCStreamer.h:126
@@ +125,3 @@
+  /// the current section
+  void emitCurrentConstantPool();
+
----------------
emitLtorg or emitPool would better match the convention of naming after the assembly directive. At least mention the asm directives in the comment if you prefer  emitCurrentConstantPool.

================
Comment at: include/llvm/MC/MCStreamer.h:119
@@ +118,3 @@
+
+  /// addConstantPoolEntry - add a new entry to the constant pool
+  ///   for the current section and return an MCExpr that can be
----------------
Don't repeat the function name. Calling this .emitLDR would better match the convention of using the asm directive name. At least mention the directive in the comment.


================
Comment at: test/CodeGen/ARM/inlineasm-ldr-pseudo.ll:2
@@ +1,3 @@
+; PR18354
+; RUN: llc -mtriple=arm-none-linux   < %s -filetype=obj | llvm-objdump -d - | FileCheck %s
+; RUN: llc -mtriple=arm-apple-darwin < %s -filetype=obj | llvm-objdump -d - | FileCheck %s
----------------
As much as I hate to admit it, this is a case where using "llc -filetype=obj" is the correct test :-(

Our handling of inline assembly just forwards it to the output, so for now we need this.


http://llvm-reviews.chandlerc.com/D2638