public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Stephen Warren <swarren@wwwdotorg.org>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH v2 4/9] arm: add _thumb1_case_uqi to libgcc
Date: Tue, 14 Aug 2012 10:02:37 -0600	[thread overview]
Message-ID: <502A769D.9050201@wwwdotorg.org> (raw)
In-Reply-To: <CAM=4YETLS1uh8bOk4PymWGzagLh3nuBE5o-sLHCPt3xNc54oKg@mail.gmail.com>

On 08/13/2012 08:39 PM, V, Aneesh wrote:
> On Mon, Aug 13, 2012 at 5:36 PM, Allen Martin <amartin@nvidia.com> wrote:
>> On Mon, Aug 13, 2012 at 04:44:05PM -0700, Stephen Warren wrote:
>>> On 08/01/2012 02:32 PM, Allen Martin wrote:
>>>> Add function required by some thumb switch statements
>>>
>>>> diff --git a/arch/arm/lib/_thumb1_case_uqi.S b/arch/arm/lib/_thumb1_case_uqi.S
>>>
>>>> +   .force_thumb
>>>
>>> I believe that line should be removed.
>>>
>>> The issue here is that when gcc emits Thumb code to call this function,
>>> it actually emits:
>>>
>>>>   108af8:       f000 f94a       bl      108d90 <____gnu_thumb1_case_uqi_from_thumb>
>>>
>>> which is implemented as:
>>>
>>>> 00108d90 <____gnu_thumb1_case_uqi_from_thumb>:
>>>>   108d90:       4778            bx      pc
>>>>   108d92:       46c0            nop                     ; (mov r8, r8)
>>>>   108d94:       eafffde1        b       108520 <__gnu_thumb1_case_uqi>
>>>
>>> i.e. it switches to ARM mode then jumps to that function. Hence,
>>> __gnu_thumb1_case_uqi must be compiled as ARM, not as Thumb.
>>
>> The function is supposed to be thumb code, it's basically a thumb
>> optimization of a switch statement, and in the ARM case the compiler
>> just emits the code directly instead of calling into libgcc.  So it
>> doesn't really make sense for the function to be anything but thumb.
>>
>> I think the real problem is the linker incorrectly thought the code
>> was ARM so it generated a thumb to ARM interworking veneer.
>>
>> Can you try adding a ".type __gnu_thumb1_case_uqi STT_FUNC" instead?
>> I've seen cases where the assembler generates the wrong type of symbol
>> without some explicit guidance which messes up the linker's
>> interworking generator.  This might be one of those.
> 
> Yes, not having STT_FUNC or %function is troublesome. I have faced this
> before.
> 
> Maybe you should wrap the assembly functions with standard ENTRY,
> END_PROC macros from linkage.h. This will take care of STT_FUNC

Yes. In fact patch 7/9 of this series is doing exactly that for some
other assembly function.

So, the patch below fixes this problem; I see the function being called
directly without any compiler-synthesized "from_thumb" stub/trampoline
being used.

> diff --git a/arch/arm/lib/_thumb1_case_uqi.S b/arch/arm/lib/_thumb1_case_uqi.S
> index e4bb194..a4241f6 100644
> --- a/arch/arm/lib/_thumb1_case_uqi.S
> +++ b/arch/arm/lib/_thumb1_case_uqi.S
> @@ -25,10 +25,11 @@ along with this program; see the file COPYING.  If not, write to
>  the Free Software Foundation, 51 Franklin Street, Fifth Floor,
>  Boston, MA 02110-1301, USA.  */
>  
> +#include <linux/linkage.h>
> +
>         .force_thumb
>         .syntax unified
> -       .globl __gnu_thumb1_case_uqi
> -__gnu_thumb1_case_uqi:
> +ENTRY(__gnu_thumb1_case_uqi)
>         push    {r1}
>         mov     r1, lr
>         lsrs    r1, r1, #1
> @@ -38,4 +39,4 @@ __gnu_thumb1_case_uqi:
>         add     lr, lr, r1
>         pop     {r1}
>         bx      lr
> -
> +ENDPROC(__gnu_thumb1_case_uqi)

  reply	other threads:[~2012-08-14 16:02 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-01 20:32 [U-Boot] [PATCH v2 0/9] enable thumb for tegra20 Allen Martin
2012-08-01 20:32 ` [U-Boot] [PATCH v2 1/9] tools, config.mk: add binutils-version Allen Martin
2012-08-01 20:32 ` [U-Boot] [PATCH v2 2/9] arm: work around assembler bug Allen Martin
2012-08-01 20:32 ` [U-Boot] [PATCH v2 3/9] tegra20: remove inline assembly for u32 cast Allen Martin
2012-08-01 20:32 ` [U-Boot] [PATCH v2 4/9] arm: add _thumb1_case_uqi to libgcc Allen Martin
2012-08-13 23:44   ` Stephen Warren
2012-08-14  0:36     ` Allen Martin
2012-08-14  2:39       ` V, Aneesh
2012-08-14 16:02         ` Stephen Warren [this message]
2012-08-01 20:32 ` [U-Boot] [PATCH v2 5/9] arm: use thumb compatible return in arm720t Allen Martin
2012-08-01 20:32 ` [U-Boot] [PATCH v2 6/9] arm: change arm720t to armv4t Allen Martin
2012-08-01 20:32 ` [U-Boot] [PATCH v2 7/9] arm720t: add linkage macro for relocate_code Allen Martin
2012-08-01 20:32 ` [U-Boot] [PATCH v2 8/9] arm: use thumb interworking returns in libgcc Allen Martin
2012-08-01 21:11   ` V, Aneesh
2012-08-01 21:55     ` Allen Martin
2012-08-01 22:15       ` V, Aneesh
2012-08-01 22:28         ` Allen Martin
2012-08-01 20:32 ` [U-Boot] [PATCH v2 9/9] tegra20: enable thumb build Allen Martin
2012-08-13 21:21 ` [U-Boot] [PATCH v2 0/9] enable thumb for tegra20 Stephen Warren

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=502A769D.9050201@wwwdotorg.org \
    --to=swarren@wwwdotorg.org \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox