[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