[PATCH][AsmParser][MC] Allow asm parser to emit elf header flags

Rafael EspĂ­ndola rafael.espindola at gmail.com
Thu Oct 3 14:09:20 PDT 2013


+  /// emitEndOfAsmFile -

Don't repeat the name of the asm function.

+  enum MAFRelocationModelTy

Don't you want this in a more visible place? For example, llvm-objdump
should be able to dump the flags in the ELF header, no?

It looks like only MAF_RM_DEFAULT and MAF_RM_CPIC are used. Why? Can
you just use a boolean?

+  MipsMCAsmFlags()
+    : Model(MAF_RM_DEFAULT){}

The formatting is wrong, please pass this via clang-format.


+/// emitEndOfAsmFile -

Don't duplicate the name.


+  if (MipsELFStreamer *MES = dyn_cast<MipsELFStreamer>(&OutStreamer))

Can't you use a cast? When would this be null?


+#include "AsmParser/MipsAsmFlags.h"
 #include "MCTargetDesc/MipsELFStreamer.h"

Wrong include order.


+    else  if (RM != Reloc::Static) // Do nothing for Reloc::Static
+      llvm_unreachable("Unsupported relocation model for e_flags");

This can be just an assert, no?

+  // For llvm-mc. Set a group of ELF header flags

This comment should be incorrect. Both llc and llvm-mc should use the
same code path.


+#    .option pic0
+#    .set mips16

no commented code.



More information about the llvm-commits mailing list