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

[Jason Kim]

On Fri, Oct 15, 2010 at 9:04 PM, Rafael Espindola <espindola at google.com> wrote:
>> There's about 30 tests that currently depend upon the decimal output.
>> And maybe lots more in the test-suite.
>> I don't want to do that in the same patch. I'll add a simplified
>> --hex, and --dec (default) option switch the default once I clean up
>> the tests.
>
> elf-dump is only used in "make check-lit", so changing the tests to
> use hex shouldn't be hard. You can change just the script and the
> tests in the first commit. Example of a similar change:
>
> http://llvm.org/viewvc/llvm-project?view=rev&revision=113685

Okay, I just committed the convert (all numbers from elf-dump are now
hex) as r116753.

I attached a new patch that should hopefully address the other issues,

>
>
>
>        126 // Slots for keeping state for above member funcs.
>        127 MCObjectStreamer *ELFStreamer;
>        128 MCAssembler *Asm;
>
> You don't need all this. As the initialization show you get the asm
> from the streamer for example

Good call! Thanks!

>
>  Asm = &ELFStreamer->getAssembler();
>
>
> Why do you have many " char empty = 0;". If this is a standard
> constant you should put it in a enum somewhere.

The first two are size fields (4 byte). , the second is a required
null byte in the stream.
The equiv. in MC/ELF uses a 0, but in this case, the SmallVector uses
a const T& - (T is always a char in this case).
I though exactly matching types and a descriptive name would be ok..
Since they are isolated in the helper functions, I'll change it to
numeric 0. Is this acceptable?

>
> // ('S') (i.e. the -F¡classic¢ programmer¢s model)-A
>
> Corrupted file? Has some strange looking characters.

Oops. These were from the comments of the ARM manual - I'll fix these.

>
> Cheers,

Here's the patch. http://codereview.chromium.org/3770019

Thanks!

-jason

> --
> Rafael Ávila de Espíndola
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: arm-mc-elf-s06.patch4
Type: application/octet-stream
Size: 12522 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20101018/8c63909c/attachment.obj>