[llvm-commits] Proof of concept patch for unifying the .s/ELF emission of .ARM.attributes

[Jason Kim]

On Wed, Oct 6, 2010 at 9:22 AM, Rafael Espindola <espindola at google.com> wrote:
>> Yes.
>> emitAttributes() for ELF will have to do something like that. Having
>> the .ARM.attributes emit code inside the MCELFStreamer (and have it do
>> so only for ARM) was problematic for stylistic reasons. AFAIK, the
>> other MCELFStreamer
>> methods don't have this "hidden" switching back and forth, certainly
>> not special cased for a specific architecture like we need for ARM.
>> That was why I decided to keep it within the ARM specific AsmPrinter
>> class -
>
> I agree that it should be in the ARM specific code.
>
>> Please note, this is still early stage, I fully plan on carrying
>> through any refactoring as they come up. If it turns out that both the
>> .s and .o Attributes emission can be nicely unified, than I will do
>> so.
>
> I just think that instead of having one function that emits all
> attributes for ELF, another that emits all attributes for text (and
> one for MachO?) you should create a function that emits *one*
> attribute for any output format and the current calls to emitRawText
> just get converted to use the new method.

Agreed. That certainly should be the end goal, but one step at a time!

I'll unify the attribute emission once I get the ELF part working -
Its kind of hard to get to the end when I can't take the first step
yet :-)

The "wrapper"portion still need to be there - the .s parts don't need
a container, but the ELF parts do. So there needs to be a
emitAttributes() (plural s) and an emitAttribute() (singular). The
former is a no-op for the .s, but for ELF, it needs to set up the
..ARM.attributes section for later emission (after .text)


> Cheers,
> --
> Rafael Ávila de Espíndola
>