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

Hi,

We are trying to use clang as a drop-in replacement for the gcc come with
Android NDK. I found that MC/ELF doesn't not handle Thumb functions
properly,
e.g., bit 0 of the function name in the .symtab is not set to 1, and some thumb
instructions are not generated correctly, e.g., the addresses for tBL/tBLX are
not calculated right.

With that attached patch, we can compile and run some (not all) Android
NDK samples without problem.

The attached initial patch
1. sets bit 0 of the function address of thumb function in .symtab
   ("T is 1 if the target symbol S has type STT_FUNC and the
   symbol addresses a Thumb instruction ;it is 0 otherwise."
   from "ELF for the ARM Architecture" 4.7.1.2)

2. fixes target address tBL and tBLX
3. sets relocation type of tBL/tBLX to R_ARM_THM_CALL
    (4.7.1.6)
4. adds some attributes to attribute section when cpu is "xscale"
   (this is what used in Android NDK, when architecture is ARMv5)

--
// koan-sin tan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: llvm-thumb-elf-hack.diff3
Type: application/octet-stream
Size: 6025 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20110513/81a8e782/attachment.obj>

On 13 May 2011 06:42, Koan-Sin Tan <koansin.tan at gmail.com>
wrote:
> With that attached patch, we can compile and run some (not all) Android
> NDK samples without problem.
Hi Koan,

Have you tried to run Intel and other platform tests?

I feel uncomfortable with so many changes in generic MC/ELF regarding
specific ARM support... not to mention the magic variables on
shifts... ;)

cheers,
--renato

Hi Renato,

  Nope, I didn't run other platform tests. I'll do it. Thanks :-)
  The reason of doing ARM specific thing in generic MC/ELF
is that I don't know platform specific place to make changes.


On Fri, May 13, 2011 at 5:14 PM, Renato Golin <renato.golin at arm.com>
wrote:
> On 13 May 2011 06:42, Koan-Sin Tan <koansin.tan at gmail.com> wrote:
>> With that attached patch, we can compile and run some (not all) Android
>> NDK samples without problem.
>
> Hi Koan,
>
> Have you tried to run Intel and other platform tests?
>
> I feel uncomfortable with so many changes in generic MC/ELF regarding
> specific ARM support... not to mention the magic variables on
> shifts... ;)
>
> cheers,
> --renato
>


-- 
// koan-sin tan

Renato,

  The result of running 'llvm-lit $LLVM/TEST/MC" is:
Expected Passes : 330
Expected Failures : 20
 Actually it passed "make check", no unexpected result.

On Fri, May 13, 2011 at 5:14 PM, Renato Golin <renato.golin at arm.com>
wrote:
> On 13 May 2011 06:42, Koan-Sin Tan <koansin.tan at gmail.com> wrote:
>> With that attached patch, we can compile and run some (not all) Android
>> NDK samples without problem.
>
> Hi Koan,
>
> Have you tried to run Intel and other platform tests?
>
> I feel uncomfortable with so many changes in generic MC/ELF regarding
> specific ARM support... not to mention the magic variables on
> shifts... ;)
>
> cheers,
> --renato
>


-- 
// koan-sin tan

Hi Koan,

In general, this looks OK to me. Please split the patch into separate pieces,
one for each issue you're addressing, though. From your description, it
sounds like this should be 4 patches. That way we have a cleaner revision
history in svn.

-Jim

On May 12, 2011, at 10:42 PM, Koan-Sin Tan wrote:
> Hi,
> 
> We are trying to use clang as a drop-in replacement for the gcc come with
> Android NDK. I found that MC/ELF doesn't not handle Thumb functions
properly,
> e.g., bit 0 of the function name in the .symtab is not set to 1, and some
thumb
> instructions are not generated correctly, e.g., the addresses for tBL/tBLX
are
> not calculated right.
> 
> With that attached patch, we can compile and run some (not all) Android
> NDK samples without problem.
> 
> The attached initial patch
> 1. sets bit 0 of the function address of thumb function in .symtab
>   ("T is 1 if the target symbol S has type STT_FUNC and the
>   symbol addresses a Thumb instruction ;it is 0 otherwise."
>   from "ELF for the ARM Architecture" 4.7.1.2)
> 
> 2. fixes target address tBL and tBLX
> 3. sets relocation type of tBL/tBLX to R_ARM_THM_CALL
>    (4.7.1.6)
> 4. adds some attributes to attribute section when cpu is "xscale"
>   (this is what used in Android NDK, when architecture is ARMv5)
> 
> --
> // koan-sin tan
>
<llvm-thumb-elf-hack.diff3>_______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

Jim, 

Thanks, I'll do it accordingly.

Sent from my iPhone

On May 13, 2011, at 11:04 PM, Jim Grosbach <grosbach at apple.com> wrote:
> Hi Koan,
> 
> In general, this looks OK to me. Please split the patch into separate
pieces, one for each issue you're addressing, though. From your description,
it sounds like this should be 4 patches. That way we have a cleaner revision
history in svn.
> 
> -Jim
> 
> On May 12, 2011, at 10:42 PM, Koan-Sin Tan wrote:
> 
>> Hi,
>> 
>> We are trying to use clang as a drop-in replacement for the gcc come
with
>> Android NDK. I found that MC/ELF doesn't not handle Thumb functions
properly,
>> e.g., bit 0 of the function name in the .symtab is not set to 1, and
some thumb
>> instructions are not generated correctly, e.g., the addresses for
tBL/tBLX are
>> not calculated right.
>> 
>> With that attached patch, we can compile and run some (not all) Android
>> NDK samples without problem.
>> 
>> The attached initial patch
>> 1. sets bit 0 of the function address of thumb function in .symtab
>>  ("T is 1 if the target symbol S has type STT_FUNC and the
>>  symbol addresses a Thumb instruction ;it is 0 otherwise."
>>  from "ELF for the ARM Architecture" 4.7.1.2)
>> 
>> 2. fixes target address tBL and tBLX
>> 3. sets relocation type of tBL/tBLX to R_ARM_THM_CALL
>>   (4.7.1.6)
>> 4. adds some attributes to attribute section when cpu is
"xscale"
>>  (this is what used in Android NDK, when architecture is ARMv5)
>> 
>> --
>> // koan-sin tan
>>
<llvm-thumb-elf-hack.diff3>_______________________________________________
>> LLVM Developers mailing list
>> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>

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

On Fri, May 13, 2011 at 1:42 PM, Koan-Sin Tan <koansin.tan at gmail.com>
wrote:
> Hi,
>
> We are trying to use clang as a drop-in replacement for the gcc come with
> Android NDK. I found that MC/ELF doesn't not handle Thumb functions
properly,
> e.g., bit 0 of the function name in the .symtab is not set to 1, and some
thumb
> instructions are not generated correctly, e.g., the addresses for tBL/tBLX
are
> not calculated right.
>
> With that attached patch, we can compile and run some (not all) Android
> NDK samples without problem.
>
> The attached initial patch
> 1. sets bit 0 of the function address of thumb function in .symtab
>   ("T is 1 if the target symbol S has type STT_FUNC and the
>   symbol addresses a Thumb instruction ;it is 0 otherwise."
>   from "ELF for the ARM Architecture" 4.7.1.2)
>
> 2. fixes target address tBL and tBLX
> 3. sets relocation type of tBL/tBLX to R_ARM_THM_CALL
>    (4.7.1.6)
> 4. adds some attributes to attribute section when cpu is "xscale"
>   (this is what used in Android NDK, when architecture is ARMv5)
>
> --
> // koan-sin tan
>


-- 
// koan-sin tan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mcelf-thumb.tgz
Type: application/x-gzip
Size: 3785 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20110516/e574675b/attachment.bin>

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