Re: [patch] fix PR43602 for emutls targets.

[Jakub Jelinek <jakub at redhat dot com>]

On Tue, May 11, 2010 at 08:19:51PM -0400, Jack Howarth wrote:
> On Tue, May 11, 2010 at 10:13:50AM -0400, Diego Novillo wrote:
>   It doesn't appear that Jakub's suggested reconfiguration of the patch is
> going to bootstrap. If I move emutls_finish() in place of emutls_finalize_control_vars()
> in the original patch and call it from within emutls_finish() instead, the bootstrap
> fails due to...
> 
> /sw/src/fink.build/gcc46-4.6.0-1000/darwin_objdir/./prev-gcc/xgcc -B/sw/src/fink.build/gcc46-4.6.0-1000/darwin_objdir/./prev-gcc/ -B/sw/lib/gcc4.6/x86_64-apple-darwin10.3.0/bin/ -B/sw/lib/gcc4.6/x86_64-apple-darwin10.3.0/bin/ -B/sw/lib/gcc4.6/x86_64-apple-darwin10.3.0/lib/ -isystem /sw/lib/gcc4.6/x86_64-apple-darwin10.3.0/include -isystem /sw/lib/gcc4.6/x86_64-apple-darwin10.3.0/sys-include    -c  -g -O2 -gtoggle -DIN_GCC   -W -Wall -Wwrite-strings -Wcast-qual -Wstrict-prototypes -Wmissing-prototypes -Wmissing-format-attribute -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -Werror -Wold-style-definition -Wc++-compat -fno-common  -DHAVE_CONFIG_H -I. -I. -I../../gcc-4.6-20100511/gcc -I../../gcc-4.6-20100511/gcc/. -I../../gcc-4.6-20100511/gcc/../include -I../../gcc-4.6-20100511/gcc/../libcpp/include -I/sw/include -I/sw/include  -I../../gcc-4.6-20100511/gcc/../libdecnumber -I../../gcc-4.6-20100511/gcc/../libdecnumber/dpd -I../libdecnumber -I/sw/include  -I/sw/include -DCLOOG_PPL_BACKEND   -I/sw/include ../../gcc-4.6-20100511/gcc/final.c -o final.o
> ../../gcc-4.6-20100511/gcc/expmed.c: In function 'init_expmed':
> ../../gcc-4.6-20100511/gcc/expmed.c:158:3: warning: array subscript is above array bounds [-Warray-bounds]
> ../../gcc-4.6-20100511/gcc/expmed.c:165:3: warning: array subscript is above array bounds [-Warray-bounds]
> ../../gcc-4.6-20100511/gcc/expmed.c:169:3: warning: array subscript is above array bounds [-Warray-bounds]
> ../../gcc-4.6-20100511/gcc/expmed.c:173:3: warning: array subscript is above array bounds [-Warray-bounds]
> ../../gcc-4.6-20100511/gcc/expmed.c:177:3: warning: array subscript is above array bounds [-Warray-bounds]
> ../../gcc-4.6-20100511/gcc/expmed.c:181:3: warning: array subscript is above array bounds [-Warray-bounds]
> ../../gcc-4.6-20100511/gcc/expmed.c:188:3: warning: array subscript is above array bounds [-Warray-bounds]
> ../../gcc-4.6-20100511/gcc/expmed.c:204:3: warning: array subscript is above array bounds [-Warray-bounds]
> ../../gcc-4.6-20100511/gcc/expmed.c:208:3: warning: array subscript is above array bounds [-Warray-bounds]
> ../../gcc-4.6-20100511/gcc/expmed.c:212:3: warning: array subscript is above array bounds [-Warray-bounds]
> ../../gcc-4.6-20100511/gcc/expmed.c:255:8: warning: array subscript is above array bounds [-Warray-bounds]
> ../../gcc-4.6-20100511/gcc/expmed.c:270:8: warning: array subscript is above array bounds [-Warray-bounds]
> ../../gcc-4.6-20100511/gcc/expmed.c:271:8: warning: array subscript is above array bounds [-Warray-bounds]

I fail to see how the above could be related to the patch, GCC itself
doesn't use any TLS vars...

> Index: gcc/toplev.c
> ===================================================================
> --- gcc/toplev.c        (revision 159247)
> +++ gcc/toplev.c        (working copy)
> @@ -1067,6 +1067,10 @@
>    if (errorcount || sorrycount)
>      return;
>  
> +  /* Likewise for emulated thread-local storage.  */

Likewise doesn't make sense at this spot.

> +  if (!targetm.have_tls)
> +    emutls_finish ();
> +
>    varpool_assemble_pending_decls ();
>    finish_aliases_2 ();
>  
> @@ -1074,10 +1078,6 @@
>    if (flag_mudflap)
>      mudflap_finish_file ();
>  
> -  /* Likewise for emulated thread-local storage.  */
> -  if (!targetm.have_tls)
> -    emutls_finish ();
> -
>    output_shared_constant_pool ();
>    output_object_blocks ();
>  
> @@ -449,9 +451,35 @@
>    return 1;
>  }
>  
> +static int
> +emutls_finalize_control_vars_1 (void **loc, void *unused ATTRIBUTE_UNUSED)
> +{
> +  /* TODO: Add comment */

The comment should be added before the function.

> +  tree to;

No need for this extra var.

> +  struct tree_map *h = *(struct tree_map **) loc;
> +  if (h != NULL)
> +    {
> +      to = h->to;
> +      varpool_finalize_decl (to);

Just use h->to here.  And remove even the {}s.

> +    }
> +  return 1;
> +}
> +
> +void emutls_finalize_control_vars (void)

void above should go on a separate line, so that
emutls_finalize_control_vars functio name is at the beginning of the line.

> +{
> +  /* TODO: Add comment */
> +  if (emutls_htab == NULL)
> +    return;
> +
> +  htab_traverse_noresize (emutls_htab, emutls_finalize_control_vars_1, NULL);
> +}
> +
>  void
>  emutls_finish (void)
>  {
> +  /* Make sure control vars are finalized.  */
> +  emutls_finalize_control_vars();

Missing space before (.
> +
>    if (targetm.emutls.register_common)
>      {
>        tree body = NULL_TREE;
> 
> It appears that we will have to stick with the original configuration
> of the patch.

	Jakub