[PATCH] Fix ARM TLS handling (PR target/58595)

Ramana Radhakrishnan ramana.gcc@googlemail.com
Thu Mar 6 05:44:00 GMT 2014


On Wed, Mar 5, 2014 at 9:12 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> arm_legitimize_address may be called with a TLS symbol referenced, even when
> x is not itself a SYMBOL_REF.  Most often it is something like:
> (const:SI (plus:SI (symbol_ref:SI "tlsvar") (const_int NNN)))
> but generally it can be something else (e.g. from expansion of
> TARGET_MEM_REF).  Unfortunately this function legitimizes only the
> SYMBOL_REF TLS case as TLS load, but the more complex forms with e.g. -fpic
> can e.g. fall thru into legitimize_pic_address -> gen_calculate_pic_address,
> which means either wrong-code or ICE later on.
> Fixed by legitimizing both the SYMBOL_REF and
> CONST+PLUS+SYMBOL_REF+CONST_INT case, and for more complex TLS containing
> expressions just returning x (in that case the caller will split the PLUS
> on its own).
>
> Kyrill has kindly bootstrapped/regtested this on Chromebook, ok for
> trunk/4.8?
>
> 2014-03-05  Jakub Jelinek  <jakub@redhat.com>
>             Meador Inge  <meadori@codesourcery.com>
>
>         PR target/58595
>         * config/arm/arm.c (arm_tls_symbol_p): Remove.
>         (arm_legitimize_address): Call legitimize_tls_address for any
>         arm_tls_referenced_p expression, handle constant addend.  Call it
>         before testing for !TARGET_ARM.
>         (thumb_legitimize_address): Don't handle arm_tls_symbol_p here.
>
>         * gcc.dg/tls/pr58595.c: New test.


OK if no regressions. Please let any auto testers catch any issues and
back port to 4.8 in about 48 hours.

Regards,
Ramana
>
> --- gcc/config/arm/arm.c.jj     2014-03-04 08:51:39.620081210 +0100
> +++ gcc/config/arm/arm.c        2014-03-04 14:06:26.776277688 +0100
> @@ -235,7 +235,6 @@ static tree arm_gimplify_va_arg_expr (tr
>  static void arm_option_override (void);
>  static unsigned HOST_WIDE_INT arm_shift_truncation_mask (enum machine_mode);
>  static bool arm_cannot_copy_insn_p (rtx);
> -static bool arm_tls_symbol_p (rtx x);
>  static int arm_issue_rate (void);
>  static void arm_output_dwarf_dtprel (FILE *, int, rtx) ATTRIBUTE_UNUSED;
>  static bool arm_output_addr_const_extra (FILE *, rtx);
> @@ -7336,6 +7335,32 @@ legitimize_tls_address (rtx x, rtx reg)
>  rtx
>  arm_legitimize_address (rtx x, rtx orig_x, enum machine_mode mode)
>  {
> +  if (arm_tls_referenced_p (x))
> +    {
> +      rtx addend = NULL;
> +
> +      if (GET_CODE (x) == CONST && GET_CODE (XEXP (x, 0)) == PLUS)
> +       {
> +         addend = XEXP (XEXP (x, 0), 1);
> +         x = XEXP (XEXP (x, 0), 0);
> +       }
> +
> +      if (GET_CODE (x) != SYMBOL_REF)
> +       return x;
> +
> +      gcc_assert (SYMBOL_REF_TLS_MODEL (x) != 0);
> +
> +      x = legitimize_tls_address (x, NULL_RTX);
> +
> +      if (addend)
> +       {
> +         x = gen_rtx_PLUS (SImode, x, addend);
> +         orig_x = x;
> +       }
> +      else
> +       return x;
> +    }
> +
>    if (!TARGET_ARM)
>      {
>        /* TODO: legitimize_address for Thumb2.  */
> @@ -7344,9 +7369,6 @@ arm_legitimize_address (rtx x, rtx orig_
>        return thumb_legitimize_address (x, orig_x, mode);
>      }
>
> -  if (arm_tls_symbol_p (x))
> -    return legitimize_tls_address (x, NULL_RTX);
> -
>    if (GET_CODE (x) == PLUS)
>      {
>        rtx xop0 = XEXP (x, 0);
> @@ -7459,9 +7481,6 @@ arm_legitimize_address (rtx x, rtx orig_
>  rtx
>  thumb_legitimize_address (rtx x, rtx orig_x, enum machine_mode mode)
>  {
> -  if (arm_tls_symbol_p (x))
> -    return legitimize_tls_address (x, NULL_RTX);
> -
>    if (GET_CODE (x) == PLUS
>        && CONST_INT_P (XEXP (x, 1))
>        && (INTVAL (XEXP (x, 1)) >= 32 * GET_MODE_SIZE (mode)
> @@ -7756,20 +7775,6 @@ thumb_legitimize_reload_address (rtx *x_
>
>  /* Test for various thread-local symbols.  */
>
> -/* Return TRUE if X is a thread-local symbol.  */
> -
> -static bool
> -arm_tls_symbol_p (rtx x)
> -{
> -  if (! TARGET_HAVE_TLS)
> -    return false;
> -
> -  if (GET_CODE (x) != SYMBOL_REF)
> -    return false;
> -
> -  return SYMBOL_REF_TLS_MODEL (x) != 0;
> -}
> -
>  /* Helper for arm_tls_referenced_p.  */
>
>  static int
> --- gcc/testsuite/gcc.dg/tls/pr58595.c.jj       2014-03-04 12:56:48.337915114 +0100
> +++ gcc/testsuite/gcc.dg/tls/pr58595.c  2014-03-04 12:56:48.337915114 +0100
> @@ -0,0 +1,28 @@
> +/* PR target/58595 */
> +/* { dg-do run } */
> +/* { dg-options "-O2" } */
> +/* { dg-additional-options "-fpic" { target fpic } } */
> +/* { dg-require-effective-target tls } */
> +/* { dg-require-effective-target sync_int_long } */
> +
> +struct S { unsigned long a, b; };
> +__thread struct S s;
> +void bar (unsigned long *);
> +
> +__attribute__((noinline)) void
> +foo (void)
> +{
> +  int i;
> +  for (i = 0; i < 10; i++)
> +    __sync_fetch_and_add (&s.b, 1L);
> +}
> +
> +int
> +main ()
> +{
> +  s.b = 12;
> +  foo ();
> +  if (s.b != 22)
> +    __builtin_abort ();
> +  return 0;
> +}
>
>         Jakub



More information about the Gcc-patches mailing list