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

[Jack Carter]

Thanks for the review!

If we fix these things is it good to go?

Jack
________________________________________
From: Rafael Espíndola [rafael.espindola at gmail.com]
Sent: Thursday, October 03, 2013 2:09 PM
To: Jack Carter
Cc: Vladimir Medic; llvm-commits at cs.uiuc.edu; Jim Grosbach; Rich Fuhler
Subject: Re: [PATCH][AsmParser][MC] Allow asm parser to emit elf header flags

+  /// 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.