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

Rafael Espindola espindola at google.com
Tue Oct 12 07:53:40 PDT 2010


> 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
the option, I would suggest just having a global variable with 'int'
or 'hex' and FormatOutput can then just be a plain function.

> 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 :-)
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? That is a long test filename btw.

+  // Fixme: All this is duplicated in MCSectionELF. Why??
+  // Exception Index table

Is it? I cannot find any references to EXIDX for example.

How much work is it to complete WriteNopData? If you write the test in
assembly, can the assert stay?

+    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...


> As always, feedback/corrections are very welcome.
>
> -jason

Thanks,
-- 
Rafael Ávila de Espíndola




More information about the llvm-commits mailing list