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

[Jim Grosbach]

Looks OK to me. Please commit!

On Oct 11, 2010, at 11:28 AM, Jason Kim wrote:

> Hi everyone, please find enclosed updated version of arm-mc-elf-s05.patch.
> 
> On Mon, Oct 11, 2010 at 10:39 AM, Jason Kim <jasonwkim at google.com> wrote:
>> On Mon, Oct 11, 2010 at 9:20 AM, Jim Grosbach <grosbach at apple.com> wrote:
>>> Hi Jason,
>>> 
>>> Glad to see this making progress. Here's a bit of general feedback. I'm not an expert on Linux ELF, so I can't really comment on that aspect of things, though.
>>> 
>>>>  namespace ARMBuildAttrs {
>>>>    enum AttrType {
>>>> +    // For the .cpu asm construct
>>>> +    ARM_CPU,
>>>> +    // Rest correspond to ELF/.ARM.attributes
>>>>      File                      = 1,
>>>>      Section                   = 2,
>>>>      Symbol                    = 3,
>>> 
>>> 
>>> Is this new attr distinct from the ones already there for the CPU? CPU_name, in particular, seems a likely candidate. It's hard to tell since the uses of the others don't appear to be implemented yet. In any case, if a new attr does need to be added, it should be consistent with the rest of them. No prefix, and just add the value onto the end (i.e., "Cpu = 71") of the list.
>> 
>> All of the other enums in the ARMBuildAttrs are specific part of (i.e.
>> can appear as markers in) the .ARM.attributes section.
>> The new one I added correspond to the .cpu asm attr, which translates
>> into one or more sequence of CPU_NAME, CPU_ARCH ... in the actual ELF
>> section, so I needed a way to get that done.
>> There is no neat correspondence between the asm attrs and what goes
>> into the .ARM.attributes. Certainly not for the .cpu attribute (its
>> one to many)
>> 
> 
> Jim, it turns out that the bulk of the attr emission is driven by a
> bunch of bool flags from TargetMachine.
> The individual ARMBuildAttrs::AttrType flags are not selected individually,
> so there wasn't a real use for the llvm style switch except for the
> new flag I added.  (please see new comments in patch)
> 
> <cut>
>> 
>> p.s. I have another patch waiting for this to land - I will be adding
>> just enough to get a basic two instruction function (thanks for new
>> scaffolding on EncodeInstuction()  Jim!)  output as ELF .o, and be
>> somewhat sane -
>> i.e. ARM/MC/ELF hello world ("misspelled, and broken" but at least it
>> will be a start. :-)
>> 
>> 
>>> 
>>> Regards,
>>> 
>>>  Jim
>>> 
>>> 
>>> 
>>> On Oct 7, 2010, at 8:54 PM, Jason Kim wrote:
>>> 
>>>> Second set of ARM/MC/ELF changes.
>>>> 
>>>> Added ARM specific ELF section types.
>>>> Added AttributesSection to ARMElfTargetObject
>>>> First step in unifying .cpu assembly tag with ELF/.o
>>>> llc now asserts on actual ELF emission on -filetype=obj :-)
>>>> 
>>>> Feedback, please!
>>>> 
>>>> Thanks for reading!
>>>> 
>>>> -Jason
>>>> <arm-mc-elf-s05.patch>
>>> 
>>> 
>> 
> 
> Unless there is strong objection, I'd like to land this patch soon
> (today if possible).
> Thanks for reading!!
> 
> -jason
> <arm-mc-elf-s05.patch2>