Re: [PATCH: PR target/42671] Use Thumb1 GOT address loading sequence for Thumb2

[Carrot Wei <carrot at google dot com>]

Function legitimize_tls_address and arm_call_tls_get_addr contains
similar code. I modified them and ran the same tests without any new
failure. Is there any other place?


thanks
Guozhi

Index: arm.c
===================================================================
--- arm.c       (revision 156256)
+++ arm.c       (working copy)
@@ -5150,8 +5150,7 @@ arm_load_pic_register (unsigned long sav
           }

         emit_insn (gen_pic_load_addr_thumb2 (pic_reg, pic_rtx));
-         emit_insn (gen_pic_load_dot_plus_four (pic_tmp, labelno));
-         emit_insn (gen_addsi3 (pic_reg, pic_reg, pic_tmp));
+         emit_insn (gen_pic_add_dot_plus_four (pic_reg, pic_reg, labelno));
       }
      else /* TARGET_THUMB1 */
       {
@@ -5807,16 +5806,7 @@ arm_call_tls_get_addr (rtx x, rtx reg, r

  if (TARGET_ARM)
    emit_insn (gen_pic_add_dot_plus_eight (reg, reg, labelno));
-  else if (TARGET_THUMB2)
-    {
-      rtx tmp;
-      /* Thumb-2 only allows very limited access to the PC.  Calculate
-        the address in a temporary register.  */
-      tmp = gen_reg_rtx (SImode);
-      emit_insn (gen_pic_load_dot_plus_four (tmp, labelno));
-      emit_insn (gen_addsi3(reg, reg, tmp));
-    }
-  else /* TARGET_THUMB1 */
+  else /* TARGET_THUMB */
    emit_insn (gen_pic_add_dot_plus_four (reg, reg, labelno));

  *valuep = emit_library_call_value (get_tls_get_addr (), NULL_RTX,
LCT_PURE, /* LCT_CONST?  */
@@ -5870,17 +5860,7 @@ legitimize_tls_address (rtx x, rtx reg)

      if (TARGET_ARM)
       emit_insn (gen_tls_load_dot_plus_eight (reg, reg, labelno));
-      else if (TARGET_THUMB2)
-       {
-         rtx tmp;
-         /* Thumb-2 only allows very limited access to the PC.  Calculate
-            the address in a temporary register.  */
-         tmp = gen_reg_rtx (SImode);
-         emit_insn (gen_pic_load_dot_plus_four (tmp, labelno));
-         emit_insn (gen_addsi3(reg, reg, tmp));
-         emit_move_insn (reg, gen_const_mem (SImode, reg));
-       }
-      else
+      else  /* TARGET_THUMB  */
       {
         emit_insn (gen_pic_add_dot_plus_four (reg, reg, labelno));
         emit_move_insn (reg, gen_const_mem (SImode, reg));


Index: arm.md
===================================================================
--- arm.md      (revision 156256)
+++ arm.md      (working copy)
@@ -5267,7 +5267,7 @@
                   (const_int 4)
                   (match_operand 2 "" "")]
                  UNSPEC_PIC_BASE))]
-  "TARGET_THUMB1"
+  "TARGET_THUMB"
  "*
  (*targetm.asm_out.internal_label) (asm_out_file, \"LPIC\",
                                    INTVAL (operands[2]));

On Tue, Jan 26, 2010 at 7:52 PM, Richard Earnshaw <rearnsha@arm.com> wrote:
> On Tue, 2010-01-26 at 10:02 +0800, Carrot Wei wrote:
>> Could any arm maintainer help to review this small patch?
>>
>
> This is only part of the problem -- there's all the TLS code to deal
> with as well; and there's a lot of redundant code by the looks of it
> that really needs to be cleaned up.
>
> I'll try and find some time to take a look at it, but it probably won't
> be today.
>
> R.
>
>> thanks a lot
>> Guozhi
>>
>> On Sat, Jan 16, 2010 at 1:19 PM, Carrot Wei <carrot@google.com> wrote:
>> > After several days try, I successfully setup the testing with a qemu emulator.
>> >
>> > Now the compiler is configured with options --with-arch=armv7-a
>> > --target=arm-unknown-linux-gnueabi and glibc is used. -mthumb is also
>> > provided for dejagnu. There is still no new failure with my patch.
>> >
>> > thanks
>> > Guozhi
>> >
>> > On Tue, Jan 12, 2010 at 6:14 PM, Ramana Radhakrishnan
>> > <ramana.radhakrishnan@arm.com> wrote:
>> >>
>> >>> This patch was applied to trunk GCC and tested on the arm emulator with newlib.
>> >>> No new failure.
>> >>
>> >> I have no rights to approve or reject this patch but a couple of points
>> >> were not explicit in your mail.
>> >>
>> >> Did you specifically test for Thumb2 by configuring your compiler with
>> >> --with-arch=armv7-a and with -mthumb as a dejagnu variant ?
>> >>
>> >> Also I'm not sure how much execution testing you get with shared
>> >> libraries in the GNU torture tests. So maybe you want to include
>> >> arm-linux-gnueabi and eglibc as a part of some of this testing for
>> >> sanity checking ? I
>> >>
>> >>
>> >> Cheers
>> >> Ramana
>> >>
>> >>>
>> >>> ChangeLog:
>> >>> 2010-01-12 ?Wei Guozhi ?<carrot@google.com>
>> >>>
>> >>> ? ? ? ? PR target/42671
>> >>> ? ? ? ? * config/arm/arm.c (arm_load_pic_register): Change the Thumb2 code
>> >>> ? ? ? ? sequence as Thumb1.
>> >>> ? ? ? ? * config/arm/arm.md: Change "TARGET_THUMB1" to "TARGET_THUMB".
>> >>>
>> >>>
>> >>> thanks
>> >>> Guozhi
>> >>>
>> >>>
>> >>> Index: arm.c
>> >>> ===================================================================
>> >>> --- arm.c ? ? ? (revision 155799)
>> >>> +++ arm.c ? ? ? (working copy)
>> >>> @@ -5150,8 +5150,7 @@ arm_load_pic_register (unsigned long sav
>> >>> ? ? ? ? ? ? }
>> >>>
>> >>> ? ? ? ? ? emit_insn (gen_pic_load_addr_thumb2 (pic_reg, pic_rtx));
>> >>> - ? ? ? ? emit_insn (gen_pic_load_dot_plus_four (pic_tmp, labelno));
>> >>> - ? ? ? ? emit_insn (gen_addsi3 (pic_reg, pic_reg, pic_tmp));
>> >>> + ? ? ? ? emit_insn (gen_pic_add_dot_plus_four (pic_reg, pic_reg, labelno));
>> >>> ? ? ? ? }
>> >>> ? ? ? ?else /* TARGET_THUMB1 */
>> >>> ? ? ? ? {
>> >>>
>> >>>
>> >>> Index: arm.md
>> >>> ===================================================================
>> >>> --- arm.md ? ? ?(revision 155799)
>> >>> +++ arm.md ? ? ?(working copy)
>> >>> @@ -5267,7 +5267,7 @@
>> >>> ? ? ? ? ? ? ? ? ? ? (const_int 4)
>> >>> ? ? ? ? ? ? ? ? ? ? (match_operand 2 "" "")]
>> >>> ? ? ? ? ? ? ? ? ? ?UNSPEC_PIC_BASE))]
>> >>> - ?"TARGET_THUMB1"
>> >>> + ?"TARGET_THUMB"
>> >>> ? ?"*
>> >>> ? ?(*targetm.asm_out.internal_label) (asm_out_file, \"LPIC\",
>> >>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?INTVAL (operands[2]));
>> >>
>> >>
>> >
>
>
>