On Wed, Mar 5, 2014 at 9:12 AM, Jakub Jelinek <ja...@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 <ja...@redhat.com> > Meador Inge <mead...@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