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

Rafael EspĂ­ndola rafael.espindola at gmail.com
Tue Oct 15 09:00:40 PDT 2013


On 15 October 2013 10:53, Vladimir Medic <Vladimir.Medic at imgtec.com> wrote:
> Hi Rafael,
> I have made a small patch that utilizes the methods you defined, and I'd kindly ask you to review it and see if our concept is right. Please note that this is not a complete patch, it's purpose is not to be committed but rather a proof of concept.
> 1. We have changed your emitMipsHackELFFlags method with emitMipsRelocModel which emits a relocation model info to elf header.

This looks like the correct design.

>There is also a helper method emitMipsELFFlags, which in case of AsmStreamer does nothing, and for ElfStreamer actual value is emitted to the output.

This part looks wrong. How would you get the correct flags when using
llvm-mc to parse an assembly file? If these flag differences are from
command line options (instead of assembly directives) then llvm-mc
should implement equivalent options to the gnu assembler ones. They
should then be mapped to flags that are passed to the target object
streamer constructor (like a isMips16 for example).

> The model enumerated values are defined in MipsTargetStreamer. They are meant to be visible from parser since it doesn't need to know which actual output is emitted and should not be aware of elf flags or possible other output values.

OK.
.
> 1. What is the relation between AsmPrinter and TargetStreamer in the lights of these changes? Is AsmPrinter just a kind of streamer or more likely a wrapper that utilizes TargetStreamer? In later case, it seems that some methods currently existing in MipaAsmParser should be moved to MipsAsmTargetStreamer.

The AsmPrinter is just a user of the streamer. It is no different in
that regard than llvm-mc's asm parser.

> 2. As the implementation of TargetStreamer grows, would it make sense to move it to a separate file? Is there any particular reason why you put it in MipsMCTargetDesc.cpp?

Sure. It was just small and convenient to put it in
MipsMCTargetDesc.cpp. It can move to its own file if you want.

Cheers,
Rafael




More information about the llvm-commits mailing list