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

> 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

On Wed, May 18, 2011 at 12:05 AM, Rafael Avila de Espindola
<rafael.espindola at gmail.com> wrote:
>> 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.
>
Done!
>> 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.
.ll -> .s test added. As far as I can tell .ll -> .s and .ll -> .o go
different paths, so I have both .ll->.s and .ll -> .o in the test case

The revised patches (with tests in .diff) are attached.

patch-2-3.diff:
  fixes target address tBL and tBLX and sets relocation type
  of tBL/tBLX to R_ARM_THM_CALL (ARM ELF 4.7.1.6)

patch-4.diff:
  adds some attributes to attribute section when cpu is "xscale"
  (this is what used in Android NDK, when architecture is ARMv5)

Thanks,
// koan-sin tan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mc-elf-thumb.tgz
Type: application/x-gzip
Size: 1973 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20110518/ca2bd16e/attachment.bin>

> .ll ->  .s test added. As far as I can tell .ll ->  .s and .ll -> 
.o go
> different paths, so I have both .ll->.s and .ll ->  .o in the test
case
>
> The revised patches (with tests in .diff) are attached.
>
> patch-2-3.diff:
>    fixes target address tBL and tBLX and sets relocation type
>    of tBL/tBLX to R_ARM_THM_CALL (ARM ELF 4.7.1.6)
>
> patch-4.diff:
>    adds some attributes to attribute section when cpu is "xscale"
>    (this is what used in Android NDK, when architecture is ARMv5)

Done. Just two nits: A single ';' is sufficient to start a comment and 
it is easier to review patches if you attach them directly to the email.
> Thanks,
> // koan-sin tan
Thanks,
Rafael