[RFC] Remove TLS symbols from section anchor blocks

[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

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