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

[Jason Kim]

The patch has been modified to immediately unify .s/ELF emission of
.ARM.attributes
I am requesting feedback,

There's more work to be done. the .cpu attribute needs to be turned
into an enum for consistency.

Thanks!
-jason

On Wed, Oct 6, 2010 at 9:37 AM, Jason Kim <jasonwkim at google.com> wrote:
> 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
>>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: arm-mc-elf-s04.patch2
Type: application/octet-stream
Size: 5104 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20101006/164120f0/attachment.obj>