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

[Andrew Pinski]

On Fri, Mar 7, 2014 at 11:56 AM, Christophe Lyon
<christophe.lyon@linaro.org> wrote:
> Hi,
>
> It seems the new test fails at execution on aarch64-none-elf (and
> aarch64_be-none-elf) using the ARM Foundation Model as simulator.


The new testcase should have been disabled for the aarch64-elf target
as it does not have TLS support.

Thanks,
Andrew Pinski

>
> Sorry, I don't have access to more detail right now.
>
> Christophe.
>
>
> On 6 March 2014 06:44, Ramana Radhakrishnan <ramana.gcc@googlemail.com> wrote:
>> 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