[PATCH] Demote EmitRawText call in AsmPrinter::EmitInlineAsm() and remove hasRawTextSupport() call

Daniel Sanders daniel.sanders at imgtec.com
Mon Feb 10 09:41:36 PST 2014



================
Comment at: include/llvm/MC/MCAsmInfo.h:309
@@ +308,3 @@
+    /// Should we use the integrated assembler?
+    bool UseIntegratedAssembler;
+
----------------
Rafael Ávila de Espíndola wrote:
> Why do we need two variables? It seems that the constructors should be able to set UseIntegratedAssembler directly and LLVMTargetMachine.cpp can then override it if requested.
> 
I was mostly following clang's lead for this. Clang doesn't seem to use the separation for much (the only bit I've found is that the default value is re-used as the default for -masm-verbose) so I'll change it to a single variable.

================
Comment at: lib/MC/MCAsmInfo.cpp:96
@@ +95,3 @@
+  // - Windows always enables the integrated assembler by default
+  //   - MCAsmInfoCOFF is handling this case, should it be MCAsmInfoMicrosoft?
+  // - MachO targets always enables the integrated assembler by default
----------------
Rafael Ávila de Espíndola wrote:
> IMHO it is the MCAsmInfoDarwin that should be named MCAsmInfoMachO. The handling of COFF and MachO are probably correct enough to not need a FIXME :-)
Great. Do you have any thoughts on the 'OS is Solaris' check? 

I think I can put it in SparcELFMCAsmInfo and X86ELFMCAsmInfo to cover most of it, but I'm not sure if there are other subclasses that need it.


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



More information about the llvm-commits mailing list