[llvm] r200051 - [Mips] TargetStreamer ELF flag Support for default and commandline options.

[Rafael Espíndola]

On 25 January 2014 15:45, Jack Carter <Jack.Carter at imgtec.com> wrote:
> I will revert this.
>
> That said, I really need to set the default and commanline flags. I want to do this for AsmParser as well as AsmPrinter.
>
> Is it just the name of the method that is bothering you?

Have you actually read the email? Have you paid any attention to all
the discussions over email and in person about what is the design of
the MCStreamer interface? You just added a method that doesn't
correspond to anything in a .s file. How can you possibly think that
the issue was just the method name!?!?

I have finished implementing the bits of the object emission that were
being hacked before (r200138). It is pretty clear you don't have a
solid understanding of this part of the code, so please refrain from
any commits before reviews in MC code from now on.

>Or is it the problem of the code generator not issuing the default and commandline directives directly out to the output file and have AsmParser never deal with commandline or default values?
>
> Is what you are saying is that I don't use a TargetStreamer interface for either? If I take out the MipsTargetSteamer part will you be happy? I don't think so because I cannot use hasRawTextSupport to discriminate between ELF and ASCII in MipsAsmPrinter.
>
> The Hack variation is not correct either. How do I set the EFLAGS from default and commandline flags at startup time?
>
> All the rest can be handled by the directives.
>
> Mips is not arm nor is it x86. It uses, possibly overuses the ELF header flags extensively for linking whereas other target use other means. Default values affect eflags, commandline options can override the default and or add/negate eflags, inline directives can add/negate eflags.
>
> I need a workable solution, not a hack. I request you help in resolving this problem.
>
> Sincerely,
>
> Jack
> ________________________________________
> From: Rafael Espíndola [rafael.espindola at gmail.com]
> Sent: Saturday, January 25, 2014 6:58 AM
> To: Jack Carter
> Cc: llvm-commits
> Subject: Re: [llvm] r200051 - [Mips] TargetStreamer ELF flag Support for default and commandline options.
>
>> +  getTargetStreamer().emitMipsELFFlags(EFlags);
>> +}
>
> This is incorrect. If there is no assembly directive that takes and
> sets eflags, there should not be any emitMipsELFFlags. At first look
> this is not better than the hack version that was there before.
>
>
>> +void MipsTargetAsmStreamer::emitMipsELFFlags(unsigned Flags) { return; }
>
> At a second look this is *far* worse. You once again has put the mips
> backend in a position where it has a different code path for text and
> object files. Please revert this.
>
> If there are common flags that are set by default when reading a .s
> file that should be implemented in the target streamer creation. It
> should *not* be part of the streamer interface.
>> +void MipsAsmPrinter::processInitialEFlags() {
>> +  // Not having this check would work too, but would have us chew through
>> +  // code that it doesn't use for RawText.
>> +  if (OutStreamer.hasRawTextSupport())
>> +    return;
>
> Adding calls to hasRawTextSupport is not acceptable.
>
> Cheers,
> Rafael
>