llvm dev - May 2011 - [LLVMdev] [Patch] Let MC/ELF generate Thumb/Thumb-2 are properly

On 11-05-16 09:21 AM, Koan-Sin Tan wrote:
> splited patches and test cases
>
> mc-elf-thumbfunction-bit.diff: for 1
> mc-elf-arm-backend-bl-blx-sign-bit.diff: for 2.
> mc-elf-thumb-bl-blx-relocation-table-entry.diff: for 3.
> mc-elf-cpu-xscale-attributes.diff: for 4.
>
> elf-thumbfunc.s: test case for 1
> elf-thumbfunc-reloc.ll: test case for 2 and 3
> elf-xscale-attribute.ll: test case for 4
>
I have checked in the first patch and test (with small modifications to 
the test).

For the next patches, can you include the tests in the patch itself? If 
using svn you can just svn add the test before running svn diff.

Can elf-thumbfunc-reloc.ll be converted to a llvm-mc (.s -> .o) test? 
That would make it a lot more resistant to changes in codegen.

Same for elf-xscale-attribute.ll (or maybe this one should be a .ll -> .s?).

Thanks,
Rafael

On Tue, May 17, 2011 at 12:29 AM, Rafael Avila de Espindola
<rafael.espindola at gmail.com> wrote:
> On 11-05-16 09:21 AM, Koan-Sin Tan wrote:
>>
>> splited patches and test cases
>>
>> mc-elf-thumbfunction-bit.diff: for 1
>> mc-elf-arm-backend-bl-blx-sign-bit.diff: for 2.
>> mc-elf-thumb-bl-blx-relocation-table-entry.diff: for 3.
>> mc-elf-cpu-xscale-attributes.diff: for 4.
>>
>> elf-thumbfunc.s: test case for 1
>> elf-thumbfunc-reloc.ll: test case for 2 and 3
>> elf-xscale-attribute.ll: test case for 4
>>
>
> I have checked in the first patch and test (with small modifications to the
> test).
>
> For the next patches, can you include the tests in the patch itself? If
> using svn you can just svn add the test before running svn diff.
>
> Can elf-thumbfunc-reloc.ll be converted to a llvm-mc (.s -> .o) test?
That
> would make it a lot more resistant to changes in codegen.
>
> Same for elf-xscale-attribute.ll (or maybe this one should be a .ll ->
.s?).
>
Thanks for the review and checkin.

Regarding elf-thumbfunc-reloc.ll, it seems to me that current ARMAsmParser
doesn't recognize "(PLT)", so something like  "bl
foo(PLT)" doesn't work
consequently. And I don't know how to write .s to test this without (PLT).

Regarding elf-xscale-attribute.ll, those attributes are for
".eabi_attribute" lines
in .s files and attributes in .aeabi section in .o files. Both .ll->.s
and .ll->.o
should be tested. Surely, not .s -> .o.  I didn't write .ll -> .s
because
ARMAsmParser ignore ".eabi_attribute" for now

-- 
// koan-sin tan

> Thanks for the review and checkin.
Thanks for the patch!
> Regarding elf-thumbfunc-reloc.ll, it seems to me that current ARMAsmParser
> doesn't recognize "(PLT)", so something like  "bl
foo(PLT)" doesn't work
> consequently. And I don't know how to write .s to test this without
(PLT).
NP. Can you just add that as a FIXME in elf-thumbfunc-reloc.ll? That way 
we know why it is there and can port it once more of the asm parser works.
> Regarding elf-xscale-attribute.ll, those attributes are for
> ".eabi_attribute" lines
> in .s files and attributes in .aeabi section in .o files. Both .ll->.s
> and .ll->.o
> should be tested. Surely, not .s ->  .o.  I didn't write .ll -> 
.s because
> ARMAsmParser ignore ".eabi_attribute" for now
Since this is something that happens in Codegen (.ll -> .s), I think 
this should probably be tested by looking at the .s produced. The fact 
that llvm-mc is still not able to process .eabi_attribute is a different 
issues which should get its own tests once it is able to.

Thanks,
Rafael