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

[Jason Kim]

On Tue, Oct 12, 2010 at 10:40 AM, Jason Kim <jasonwkim at google.com> wrote:
> On Tue, Oct 12, 2010 at 7:53 AM, Rafael Espindola <espindola at google.com> wrote:
>>> arm-mc-elf-s06-elfdump.patch modifies elf-dump script.
>>> It adds a --hex mode for printing all numbers to hex format.
>>> AFAIK, elf-dump output without --hex has remained the same.
>>
>> Not sure if I like the idea of having the option. I do like the idea
>> of printing in hex. If you have a good argument for why we should have

Oops, I misread Rafael's sentence. Sorry about that Rafael., Didn't
mean to sound flamey.
In any case, --hex preserves old behavior. It seemed like the least
intrusive way to get hex dump to work.


>
> I hope you are kidding about that! Do you like converting between hex
> and decimal just for testing?
> I added it just for sanity checking the magic constants from the arch.
> It sucks to have to convert 0x70000003 to decimal and back again, just
> for testing purposes (and then forgetting two weeks later what the
> magic number was supposed to be!
> 0x700000003 is recognizable. The decimal form isn't (unless you are strange! :-)
>
>> the option, I would suggest just having a global variable with 'int'
>> or 'hex' and FormatOutput can then just be a plain function.
>
> Hmm, yeah, might as well as do it that way - For some reason, the %
> operator seems to interfere with "%s %s" % Foo(a, b) versus "%s"
> %Foo(a), "%s"%Foo(b),
> so having a class to do mass convert of all args didn't work.
> I'll fix it.
>
>>
>>> The second patch arm-mc-elf-s06.patch changes finally adds enough code to
>>> directly test ARM/MC/ELF/.o emission.
>>> The newly added test passes but the resulting ELF is by no means correct.
>>> It is a simple step in the right direction.
>>
>> What is not correct about it? I hope that the items you have CHECK for
>> are correct :-)
>
> Yes, except for the subsection offsets. There's also .cpu related
> cruft that is missing. As they say, one step at a time.... :-)
> I guess I should have described it as "Substantially correct, with
> additional details remaining to be worked out"
>
>> Is enough of the asm parser working for
>> 2010-10-08-mc-asm-header-obj-test.ll to be written in assembly and use
>> llvm-mc?
>
> I haven't tested it that way yet. I suppose I'll give it a shot.
> Regardless, I'd argue that it's beyond the scope of these two patches.
>
>> That is a long test filename btw.
>
> Only 3 chars longer than 2010-09-29-mc-asm-header-test.ll :-)
>
>> +  // Fixme: All this is duplicated in MCSectionELF. Why??
>> +  // Exception Index table
>>
>> Is it? I cannot find any references to EXIDX for example.
>
> Much of the ELF specific flags are repeated on both files. I don't
> know why. Fixing that is beyond scope of these two patches.
>
>> How much work is it to complete WriteNopData? If you write the test in
>> assembly, can the assert stay?
>
> I think I'll separate that out into 1+ new patches - My next patch in
> sequence  was to fill out enough of the ARMMCEncode to handle mov r0,
> #0 and bx lr
> So might as well as add in Nop generation there as well
>
>> +    MCObjectStreamer *ELFStreamer;
>> +    MCAssembler *Asm;
>> +    MCSectionData *AttrSecData;
>> +    MCDataFragment *headFragment
>>
>> Do you really need all this in here? I don't see something similar in
>> X86 for example...
>
> The are not there in X86, because there are no X86 specific sections
> in the X86 ELF ABI! (to be more precise, ELF existed first for X86, so
> by definition, it need not any specific sections just for it.)
> ARM requires it. In order to add a ELF section outside of the
> MCAssembler/MCELFStreamer, I needed to have those intermediates there
> anyway. Either regenerate them every time I need it, or save it. I
> chose the latter.
> MC seem to do a good job of unifying between ELF/COFF/MachO, but not
> everything can be neatly lifted that way. An ARCH specific section
> seems to be one of these.
>
>>
>>> As always, feedback/corrections are very welcome.
>>>
>>> -jason
>>
>> Thanks,
>> --
>> Rafael Ávila de Espíndola
>>
>
>
> So I guess I'll do the following.
> 1. Fix up elf-dump
> 2. Fix up Nop generation
> 3. Reland this patch in sequence.
>
> Does this sound like a sane plan?
>
>
> Thanks for feedback!
> -Jason
>