Patchwork [RFC] Remove TLS symbols from section anchor blocks

login
register
mail settings
Submitter David Edelsohn
Date Dec. 8, 2012, 6:33 p.m.
Message ID <CAGWvnykGiqaUbzU1-byaLqhdP_r47FuirbQ3G25JHTP1JASHJA@mail.gmail.com>
Download mbox | patch
Permalink /patch/204675/
State New
Headers show

Comments

David Edelsohn - Dec. 8, 2012, 6:33 p.m.
[Sorry, I forgot to copy GCC Patches before]

Some of the libgomp testcases fail on AIX when using native TLS
because variables annotated with

#pragma omp threadprivate(thr)

sometimes are placed in section anchor blocks.  Variables declared
with "__thread" create decls with the correct TLS model from the
beginning, but OpenMP sometimes creates a decl with TLS_MODEL_NONE and
later tries to make the decl again. make_decl_rtl() is called again
and it calls ENCODE_SECTION_INFO and then change_symbol_block().
However ENCODE_SECTION_INFO explicitly preserves
SYMBOL_FLAG_HAS_BLOCK_INFO and change_symbol_block() place the symbol
in a different block but never re-assesses if the new attributes of
the symbol make it in appropriate for section anchor blocks.

The following patch adds an ENCODE_SECTION_INFO hook for AIX that
removes the SYMBOL_FLAG_HAS_BLOCK_INFO flag if the symbol is
DECL_THREAD_LOCAL_P. This prevents the symbol from being emitted in a
section anchor block and it correctly is emitted in its own section;
it does not remove it from the section anchor blocks data structures.
This fixes some of the libgomp failures.

Is this a reasonable approach?  Or should change_symbol_block re-test
use_blocks_for_symbol_p()?

Thanks, David
Richard Sandiford - Dec. 10, 2012, 10:46 a.m.
David Edelsohn <dje.gcc@gmail.com> writes:
> [Sorry, I forgot to copy GCC Patches before]
>
> Some of the libgomp testcases fail on AIX when using native TLS
> because variables annotated with
>
> #pragma omp threadprivate(thr)
>
> sometimes are placed in section anchor blocks.  Variables declared
> with "__thread" create decls with the correct TLS model from the
> beginning, but OpenMP sometimes creates a decl with TLS_MODEL_NONE and
> later tries to make the decl again. make_decl_rtl() is called again
> and it calls ENCODE_SECTION_INFO and then change_symbol_block().
> However ENCODE_SECTION_INFO explicitly preserves
> SYMBOL_FLAG_HAS_BLOCK_INFO and change_symbol_block() place the symbol
> in a different block but never re-assesses if the new attributes of
> the symbol make it in appropriate for section anchor blocks.
>
> The following patch adds an ENCODE_SECTION_INFO hook for AIX that
> removes the SYMBOL_FLAG_HAS_BLOCK_INFO flag if the symbol is
> DECL_THREAD_LOCAL_P. This prevents the symbol from being emitted in a
> section anchor block and it correctly is emitted in its own section;
> it does not remove it from the section anchor blocks data structures.
> This fixes some of the libgomp failures.
>
> Is this a reasonable approach?  Or should change_symbol_block re-test
> use_blocks_for_symbol_p()?

TBH I don't really know.  The original idea was that use_blocks_for_decl_p
checks invariants and get_block_for_decl checks things that could conceivably
change between calls to make_decl_rtl (including changes that make the decl
suitable for object blocks, e.g. because we now have a definition of something
that used to be extern).

So perhaps we should check the use_blocks_for_decl_p target hook in
get_block_for_decl instead of varasm.c:use_blocks_for_decl_p.
I.e. something like:

/* Return the block into which object_block DECL should be placed.  */

static struct object_block *
get_block_for_decl (tree decl)
{
  section *sect;

  ...

  if (!targetm.use_blocks_for_decl_p (decl))
    return NULL;

  /* Find out which section should contain DECL.  We cannot put it into
     an object block if it requires a standalone definition.  */
  if (TREE_CODE (decl) == VAR_DECL)
      align_variable (decl, 0);
  sect = get_variable_section (decl, true);
  if (SECTION_STYLE (sect) == SECTION_NOSWITCH)
    return NULL;

  return get_block_for_section (sect);
}

Although it's not exactly ideal having a hook called use_blocks_for_decl_p
and a function called use_blocks_for_decl_p that do different things...

Richard

Patch

Index: rs6000.c
===================================================================
--- rs6000.c    (revision 194278)
+++ rs6000.c    (working copy)
@@ -25886,6 +25886,29 @@ 
         ? "\t.long _section_.text\n" : "\t.llong _section_.text\n",
         asm_out_file);
 }
+
+static void
+rs6000_xcoff_encode_section_info (tree decl, rtx rtl, int first)
+{
+  rtx symbol;
+  int flags;
+
+  default_encode_section_info (decl, rtl, first);
+
+  /* Careful not to prod global register variables.  */
+  if (!MEM_P (rtl))
+    return;
+  symbol = XEXP (rtl, 0);
+  if (GET_CODE (symbol) != SYMBOL_REF)
+    return;
+
+  flags = SYMBOL_REF_FLAGS (symbol);
+
+  if (TREE_CODE (decl) == VAR_DECL && DECL_THREAD_LOCAL_P (decl))
+    flags &= ~SYMBOL_FLAG_HAS_BLOCK_INFO;
+
+  SYMBOL_REF_FLAGS (symbol) = flags;
+}
 #endif /* TARGET_XCOFF */

 /* Compute a (partial) cost for rtx X.  Return true if the complete