[PATCH][C] Always use TRANSLATION_UNIT_DECL as context

[PATCH][C] Always use TRANSLATION_UNIT_DECL as context

[Richard Guenther]

This transitions the C frontend to use a TRANSLATION_UNIT_DECL
DECL_CONTEX for file-scope decls.  Fortunately all this was in
place for -combine support, I just need to disable the n_files check.

The default langhook mangling for file-scope statics needs to be
adjusted, otherwise we'll get fails from testcases that assume
global statics retain their name.  That part is fine now that
-combine is gone and no longer relies on that (LTO has it's own
handling of file-scope conflicts).

I had this bootstrapped and tested earlier, re-testing ontop of
the patch series posted.

Ok if that succeeds?

That leaves me with the C++ FE to deal with - but I'm already
quite lost there, any hints where to hook into?

Thanks,
Richard.

2010-09-17  Richard Guenther  <rguenther@suse.de>

	* c-decl.c (pop_scope): Always set file-scope DECL_CONTEXT.
	Make sure to not call set_type_context with error_mark_node.
	* langhooks.c (lhd_set_decl_assembler_name): Use DECL_FILE_SCOPE_P.

Index: gcc/c-decl.c
===================================================================
*** gcc/c-decl.c.orig	2010-09-17 12:39:59.000000000 +0200
--- gcc/c-decl.c	2010-09-17 14:11:49.000000000 +0200
*************** pop_scope (void)
*** 1203,1217 ****
  	      DECL_CHAIN (extp) = BLOCK_VARS (block);
  	      BLOCK_VARS (block) = extp;
  	    }
! 	  /* If this is the file scope, and we are processing more
! 	     than one translation unit in this compilation, set
! 	     DECL_CONTEXT of each decl to the TRANSLATION_UNIT_DECL.
! 	     This makes same_translation_unit_p work, and causes
! 	     static declarations to be given disambiguating suffixes.  */
! 	  if (scope == file_scope && num_in_fnames > 1)
  	    {
  	      DECL_CONTEXT (p) = context;
! 	      if (TREE_CODE (p) == TYPE_DECL)
  		set_type_context (TREE_TYPE (p), context);
  	    }
  
--- 1203,1216 ----
  	      DECL_CHAIN (extp) = BLOCK_VARS (block);
  	      BLOCK_VARS (block) = extp;
  	    }
! 	  /* If this is the file scope set DECL_CONTEXT of each decl to
! 	     the TRANSLATION_UNIT_DECL.  This makes same_translation_unit_p
! 	     work.  */
! 	  if (scope == file_scope)
  	    {
  	      DECL_CONTEXT (p) = context;
! 	      if (TREE_CODE (p) == TYPE_DECL
! 		  && TREE_TYPE (p) != error_mark_node)
  		set_type_context (TREE_TYPE (p), context);
  	    }
  
Index: gcc/langhooks.c
===================================================================
*** gcc/langhooks.c.orig	2010-09-17 12:39:59.000000000 +0200
--- gcc/langhooks.c	2010-09-17 13:34:00.000000000 +0200
*************** lhd_set_decl_assembler_name (tree decl)
*** 176,182 ****
       is less than the whole compilation.  Concatenate a distinguishing
       number - we use the DECL_UID.  */
  
!   if (TREE_PUBLIC (decl) || DECL_CONTEXT (decl) == NULL_TREE)
      id = targetm.mangle_decl_assembler_name (decl, DECL_NAME (decl));
    else
      {
--- 176,182 ----
       is less than the whole compilation.  Concatenate a distinguishing
       number - we use the DECL_UID.  */
  
!   if (TREE_PUBLIC (decl) || DECL_FILE_SCOPE_P (decl))
      id = targetm.mangle_decl_assembler_name (decl, DECL_NAME (decl));
    else
      {

Re: [PATCH][C] Always use TRANSLATION_UNIT_DECL as context

[Joseph S. Myers]

On Fri, 17 Sep 2010, Richard Guenther wrote:

> 	* c-decl.c (pop_scope): Always set file-scope DECL_CONTEXT.
> 	Make sure to not call set_type_context with error_mark_node.

OK.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH][C] Always use TRANSLATION_UNIT_DECL as context

[Jie Zhang]

On 09/17/2010 08:18 PM, Richard Guenther wrote:
> 	* c-decl.c (pop_scope): Always set file-scope DECL_CONTEXT.

This change causes regressions on arm-none-eabi:

FAIL: gcc.c-torture/execute/builtins/abs-1.c compilation,  -O2 -flto
UNRESOLVED: gcc.c-torture/execute/builtins/abs-1.c execution,  -O2 -flto

The compiler error messages are:

testsuite/gcc.c-torture/execute/builtins/abs-1.c:22:1: sorry, 
unimplemented: gimple bytecode streams do not support machine specific 
builtin functions on this target
testsuite/gcc.c-torture/execute/builtins/abs-1-lib.c:22:1: sorry, 
unimplemented: gimple bytecode streams do not support machine specific 
builtin functions on this target
testsuite/gcc.c-torture/execute/builtins/lib/main.c:13:1: sorry, 
unimplemented: gimple bytecode streams do not support machine specific 
builtin functions on this target
compiler exited with status 1

I did some investigation, but don't know how to fix it.

The above change makes GCC always set file-scope DECL_CONTEXT. So when 
come to

static void
lto_output_ts_decl_minimal_tree_pointers (struct output_block *ob, tree 
expr,
					  bool ref_p)
{
   lto_output_tree_or_ref (ob, DECL_NAME (expr), ref_p);
   lto_output_tree_or_ref (ob, DECL_CONTEXT (expr), ref_p);
   lto_output_location (ob, DECL_SOURCE_LOCATION (expr));
}

with EXPR being a file scope function, DECL_CONEXT (expr) is the 
translation unit which contains the function. Then

   lto_output_tree_or_ref (ob, DECL_CONTEXT (expr), ref_p);

writes out the whole translation unit including all the builtin 
functions. Finally, in lto_output_builtin_tree, a "sorry" is called with 
the above error message since ARM target does not have targetm.builtin_decl.

I don't know how to fix this. Defining targetm.builtin_decl for ARM 
should fix it. But I'm not sure if we really want to output all builtins 
for each translation unit. This regression should exist for all targets 
which don't provide targetm.builtin_decl.


-- 
Jie Zhang
CodeSourcery

Re: [PATCH][C] Always use TRANSLATION_UNIT_DECL as context

[Richard Guenther]

On Sat, 25 Sep 2010, Jie Zhang wrote:

> On 09/17/2010 08:18 PM, Richard Guenther wrote:
> > 	* c-decl.c (pop_scope): Always set file-scope DECL_CONTEXT.
> 
> This change causes regressions on arm-none-eabi:
> 
> FAIL: gcc.c-torture/execute/builtins/abs-1.c compilation,  -O2 -flto
> UNRESOLVED: gcc.c-torture/execute/builtins/abs-1.c execution,  -O2 -flto
> 
> The compiler error messages are:
> 
> testsuite/gcc.c-torture/execute/builtins/abs-1.c:22:1: sorry, unimplemented:
> gimple bytecode streams do not support machine specific builtin functions on
> this target
> testsuite/gcc.c-torture/execute/builtins/abs-1-lib.c:22:1: sorry,
> unimplemented: gimple bytecode streams do not support machine specific builtin
> functions on this target
> testsuite/gcc.c-torture/execute/builtins/lib/main.c:13:1: sorry,
> unimplemented: gimple bytecode streams do not support machine specific builtin
> functions on this target
> compiler exited with status 1
> 
> I did some investigation, but don't know how to fix it.
> 
> The above change makes GCC always set file-scope DECL_CONTEXT. So when come to
> 
> static void
> lto_output_ts_decl_minimal_tree_pointers (struct output_block *ob, tree expr,
> 					  bool ref_p)
> {
>   lto_output_tree_or_ref (ob, DECL_NAME (expr), ref_p);
>   lto_output_tree_or_ref (ob, DECL_CONTEXT (expr), ref_p);
>   lto_output_location (ob, DECL_SOURCE_LOCATION (expr));
> }
> 
> with EXPR being a file scope function, DECL_CONEXT (expr) is the translation
> unit which contains the function. Then
> 
>   lto_output_tree_or_ref (ob, DECL_CONTEXT (expr), ref_p);
> 
> writes out the whole translation unit including all the builtin functions.
> Finally, in lto_output_builtin_tree, a "sorry" is called with the above error
> message since ARM target does not have targetm.builtin_decl.
> 
> I don't know how to fix this. Defining targetm.builtin_decl for ARM should fix
> it. But I'm not sure if we really want to output all builtins for each
> translation unit. This regression should exist for all targets which don't
> provide targetm.builtin_decl.

Yes, this writes all declarations in the TRANSLATION_UNIT_DECLs BLOCK
tree.  Does the following fix it?

Index: tree.c
===================================================================
--- tree.c      (revision 164590)
+++ tree.c      (working copy)
@@ -4585,6 +4690,8 @@ free_lang_data_in_decl (tree decl)
         nesting beyond this point. */
       DECL_CONTEXT (decl) = NULL_TREE;
     }
+  else if (TREE_CODE (decl) == TRANSLATION_UNIT_DECL)
+    DECL_INITIAL (decl) = NULL_TREE;
 }
 
 

Richard.

Re: [PATCH][C] Always use TRANSLATION_UNIT_DECL as context

[Jie Zhang]

On 09/27/2010 04:49 PM, Richard Guenther wrote:
> On Sat, 25 Sep 2010, Jie Zhang wrote:
>
>> On 09/17/2010 08:18 PM, Richard Guenther wrote:
>>> 	* c-decl.c (pop_scope): Always set file-scope DECL_CONTEXT.
>>
>> This change causes regressions on arm-none-eabi:
>>
>> FAIL: gcc.c-torture/execute/builtins/abs-1.c compilation,  -O2 -flto
>> UNRESOLVED: gcc.c-torture/execute/builtins/abs-1.c execution,  -O2 -flto
>>
>> The compiler error messages are:
>>
>> testsuite/gcc.c-torture/execute/builtins/abs-1.c:22:1: sorry, unimplemented:
>> gimple bytecode streams do not support machine specific builtin functions on
>> this target
>> testsuite/gcc.c-torture/execute/builtins/abs-1-lib.c:22:1: sorry,
>> unimplemented: gimple bytecode streams do not support machine specific builtin
>> functions on this target
>> testsuite/gcc.c-torture/execute/builtins/lib/main.c:13:1: sorry,
>> unimplemented: gimple bytecode streams do not support machine specific builtin
>> functions on this target
>> compiler exited with status 1
>>
>> I did some investigation, but don't know how to fix it.
>>
>> The above change makes GCC always set file-scope DECL_CONTEXT. So when come to
>>
>> static void
>> lto_output_ts_decl_minimal_tree_pointers (struct output_block *ob, tree expr,
>> 					  bool ref_p)
>> {
>>    lto_output_tree_or_ref (ob, DECL_NAME (expr), ref_p);
>>    lto_output_tree_or_ref (ob, DECL_CONTEXT (expr), ref_p);
>>    lto_output_location (ob, DECL_SOURCE_LOCATION (expr));
>> }
>>
>> with EXPR being a file scope function, DECL_CONEXT (expr) is the translation
>> unit which contains the function. Then
>>
>>    lto_output_tree_or_ref (ob, DECL_CONTEXT (expr), ref_p);
>>
>> writes out the whole translation unit including all the builtin functions.
>> Finally, in lto_output_builtin_tree, a "sorry" is called with the above error
>> message since ARM target does not have targetm.builtin_decl.
>>
>> I don't know how to fix this. Defining targetm.builtin_decl for ARM should fix
>> it. But I'm not sure if we really want to output all builtins for each
>> translation unit. This regression should exist for all targets which don't
>> provide targetm.builtin_decl.
>
> Yes, this writes all declarations in the TRANSLATION_UNIT_DECLs BLOCK
> tree.  Does the following fix it?
>
> Index: tree.c
> ===================================================================
> --- tree.c      (revision 164590)
> +++ tree.c      (working copy)
> @@ -4585,6 +4690,8 @@ free_lang_data_in_decl (tree decl)
>           nesting beyond this point. */
>         DECL_CONTEXT (decl) = NULL_TREE;
>       }
> +  else if (TREE_CODE (decl) == TRANSLATION_UNIT_DECL)
> +    DECL_INITIAL (decl) = NULL_TREE;
>   }
>
Not yet. This change causes an ICE.

Run cc1 under GDB:

(gdb) set args -fpreprocessed main.i -quiet -dumpbase main.c -auxbase 
main -O2 -w -version -flto -fno-builtin-abs -o main.s
(gdb) r
Starting program: 
/home/jie/sources/gcc/builds/build-arm-eabi.git/gcc/cc1 -fpreprocessed 
main.i -quiet -dumpbase main.c -auxbase main -O2 -w -version -flto 
-fno-builtin-abs -o main.s
Program received signal SIGSEGV, Segmentation fault.
0x00000000004c044d in c_write_global_declarations () at 
../../../git/gcc/c-decl.c:9750
(gdb) bt
#0  0x00000000004c044d in c_write_global_declarations () at 
../../../git/gcc/c-decl.c:9750
#1  0x00000000009eb0e7 in compile_file () at ../../../git/gcc/toplev.c:967
#2  0x00000000009ed5d2 in do_compile () at ../../../git/gcc/toplev.c:2394
#3  0x00000000009ed6a0 in toplev_main (argc=15, argv=0x7fffffffe1c8) at 
../../../git/gcc/toplev.c:2435
#4  0x0000000000593d14 in main (argc=15, argv=0x7fffffffe1c8) at 
../../../git/gcc/main.c:36
(gdb) l
9745	     be emitted, output debug information for globals.  */
9746	  if (!seen_error ())
9747	    {
9748	      timevar_push (TV_SYMOUT);
9749	      FOR_EACH_VEC_ELT (tree, all_translation_units, i, t)
9750		c_write_global_declarations_2 (BLOCK_VARS (DECL_INITIAL (t)));
9751	      c_write_global_declarations_2 (BLOCK_VARS (ext_block));
9752	      timevar_pop (TV_SYMOUT);
9753	    }
9754	


-- 
Jie Zhang
CodeSourcery

Re: [PATCH][C] Always use TRANSLATION_UNIT_DECL as context

[Richard Guenther]

On Mon, 27 Sep 2010, Jie Zhang wrote:

> On 09/27/2010 04:49 PM, Richard Guenther wrote:
> > On Sat, 25 Sep 2010, Jie Zhang wrote:
> > 
> > > On 09/17/2010 08:18 PM, Richard Guenther wrote:
> > > > 	* c-decl.c (pop_scope): Always set file-scope DECL_CONTEXT.
> > > 
> > > This change causes regressions on arm-none-eabi:
> > > 
> > > FAIL: gcc.c-torture/execute/builtins/abs-1.c compilation,  -O2 -flto
> > > UNRESOLVED: gcc.c-torture/execute/builtins/abs-1.c execution,  -O2 -flto
> > > 
> > > The compiler error messages are:
> > > 
> > > testsuite/gcc.c-torture/execute/builtins/abs-1.c:22:1: sorry,
> > > unimplemented:
> > > gimple bytecode streams do not support machine specific builtin functions
> > > on
> > > this target
> > > testsuite/gcc.c-torture/execute/builtins/abs-1-lib.c:22:1: sorry,
> > > unimplemented: gimple bytecode streams do not support machine specific
> > > builtin
> > > functions on this target
> > > testsuite/gcc.c-torture/execute/builtins/lib/main.c:13:1: sorry,
> > > unimplemented: gimple bytecode streams do not support machine specific
> > > builtin
> > > functions on this target
> > > compiler exited with status 1
> > > 
> > > I did some investigation, but don't know how to fix it.
> > > 
> > > The above change makes GCC always set file-scope DECL_CONTEXT. So when
> > > come to
> > > 
> > > static void
> > > lto_output_ts_decl_minimal_tree_pointers (struct output_block *ob, tree
> > > expr,
> > > 					  bool ref_p)
> > > {
> > >    lto_output_tree_or_ref (ob, DECL_NAME (expr), ref_p);
> > >    lto_output_tree_or_ref (ob, DECL_CONTEXT (expr), ref_p);
> > >    lto_output_location (ob, DECL_SOURCE_LOCATION (expr));
> > > }
> > > 
> > > with EXPR being a file scope function, DECL_CONEXT (expr) is the
> > > translation
> > > unit which contains the function. Then
> > > 
> > >    lto_output_tree_or_ref (ob, DECL_CONTEXT (expr), ref_p);
> > > 
> > > writes out the whole translation unit including all the builtin functions.
> > > Finally, in lto_output_builtin_tree, a "sorry" is called with the above
> > > error
> > > message since ARM target does not have targetm.builtin_decl.
> > > 
> > > I don't know how to fix this. Defining targetm.builtin_decl for ARM should
> > > fix
> > > it. But I'm not sure if we really want to output all builtins for each
> > > translation unit. This regression should exist for all targets which don't
> > > provide targetm.builtin_decl.
> > 
> > Yes, this writes all declarations in the TRANSLATION_UNIT_DECLs BLOCK
> > tree.  Does the following fix it?
> > 
> > Index: tree.c
> > ===================================================================
> > --- tree.c      (revision 164590)
> > +++ tree.c      (working copy)
> > @@ -4585,6 +4690,8 @@ free_lang_data_in_decl (tree decl)
> >           nesting beyond this point. */
> >         DECL_CONTEXT (decl) = NULL_TREE;
> >       }
> > +  else if (TREE_CODE (decl) == TRANSLATION_UNIT_DECL)
> > +    DECL_INITIAL (decl) = NULL_TREE;
> >   }
> > 
> Not yet. This change causes an ICE.

Ick ...

so I guess we should instead try to not push builtins to the global
scope.  Like with

Index: c-decl.c
===================================================================
--- c-decl.c    (revision 164591)
+++ c-decl.c    (working copy)
@@ -1142,6 +1142,10 @@ pop_scope (void)
                         "inline function %q+D declared but never 
defined", p);
              DECL_EXTERNAL (p) = 1;
            }
+         else if (TREE_CODE (p) == FUNCTION_DECL
+                  && DECL_BUILT_IN (p))
+           /* Do not put builtin functions into the outermost BLOCK.  */
+           continue;
 
          goto common_symbol;
 

Joseph might have an idea here (I also see external_scope being used,
but I'm not sure what functions actually end up in that).

Does the above help?

Thanks,
Richard.

Re: [PATCH][C] Always use TRANSLATION_UNIT_DECL as context

[Jie Zhang]

On 09/27/2010 05:25 PM, Richard Guenther wrote:
> so I guess we should instead try to not push builtins to the global
> scope.  Like with
>
> Index: c-decl.c
> ===================================================================
> --- c-decl.c    (revision 164591)
> +++ c-decl.c    (working copy)
> @@ -1142,6 +1142,10 @@ pop_scope (void)
>                           "inline function %q+D declared but never
> defined", p);
>                DECL_EXTERNAL (p) = 1;
>              }
> +         else if (TREE_CODE (p) == FUNCTION_DECL
> +&&  DECL_BUILT_IN (p))
> +           /* Do not put builtin functions into the outermost BLOCK.  */
> +           continue;
>
>            goto common_symbol;
>
>
> Joseph might have an idea here (I also see external_scope being used,
> but I'm not sure what functions actually end up in that).
>
> Does the above help?
>
This change fixed the original regression. But it caused some new ICEs. Like

Run cc1 under GDB:

(gdb) set args -fpreprocessed memcpy-chk-lib.i -quiet -dumpbase 
memcpy-chk-lib.c -auxbase memcpy-chk-lib -O3 -fomit-frame-pointer -o 
memcpy-chk-lib.s
(gdb) r
Starting program: 
/home/jie/sources/gcc/builds/build-arm-eabi.git/gcc/cc1 -fpreprocessed 
memcpy-chk-lib.i -quiet -dumpbase memcpy-chk-lib.c -auxbase 
memcpy-chk-lib -O3 -fomit-frame-pointer -o memcpy-chk-lib.s

Program received signal SIGSEGV, Segmentation fault.
0x00000000004c0931 in gt_ggc_mx_c_binding (x_p=0x7ffff6f771e0) at 
./gt-c-decl.h:92
(gdb) bt
#0  0x00000000004c0931 in gt_ggc_mx_c_binding (x_p=0x7ffff6f771e0) at 
./gt-c-decl.h:92
#1  0x00000000004c3027 in gt_ggc_mx_lang_tree_node (x_p=0x7ffff6f40b90) 
at ./gt-c-decl.h:561
#2  0x00000000004c215f in gt_ggc_mx_lang_tree_node (x_p=0x7ffff6f43700) 
at ./gt-c-decl.h:370
#3  0x000000000084b4ff in gt_ggc_mx_cgraph_node (x_p=0x7ffff6f92000) at 
gtype-desc.c:524
#4  0x0000000000851920 in gt_ggc_m_P11cgraph_node4htab 
(x_p=0x7ffff6e9e930) at gtype-desc.c:2432
#5  0x00000000007e722f in ggc_mark_root_tab (rt=0x11afce0) at 
../../../git/gcc/ggc-common.c:159
#6  0x00000000007e72cb in ggc_mark_roots () at 
../../../git/gcc/ggc-common.c:178
#7  0x000000000059c45f in ggc_collect () at ../../../git/gcc/ggc-page.c:1950
#8  0x00000000008f74b9 in execute_todo (flags=2083) at 
../../../git/gcc/passes.c:1306
#9  0x00000000008f7cef in execute_one_pass (pass=0x166c180) at 
../../../git/gcc/passes.c:1592
#10 0x00000000008f7e1b in execute_pass_list (pass=0x166c180) at 
../../../git/gcc/passes.c:1624
#11 0x00000000008f7e3c in execute_pass_list (pass=0x166bfa0) at 
../../../git/gcc/passes.c:1625
#12 0x00000000008f7e3c in execute_pass_list (pass=0x166b640) at 
../../../git/gcc/passes.c:1625
#13 0x0000000000a7e6e4 in tree_rest_of_compilation 
(fndecl=0x7ffff6f2af00) at ../../../git/gcc/tree-optimize.c:419
#14 0x0000000000cf1413 in cgraph_expand_function (node=0x7ffff6d952c0) 
at ../../../git/gcc/cgraphunit.c:1477
#15 0x0000000000cf16ab in cgraph_expand_all_functions () at 
../../../git/gcc/cgraphunit.c:1556
#16 0x0000000000cf1cd0 in cgraph_optimize () at 
../../../git/gcc/cgraphunit.c:1812
#17 0x0000000000cef817 in cgraph_finalize_compilation_unit () at 
../../../git/gcc/cgraphunit.c:1020
#18 0x00000000004c044c in c_write_global_declarations () at 
../../../git/gcc/c-decl.c:9748
#19 0x00000000009eb15f in compile_file () at ../../../git/gcc/toplev.c:967
#20 0x00000000009ed64a in do_compile () at ../../../git/gcc/toplev.c:2394
#21 0x00000000009ed718 in toplev_main (argc=12, argv=0x7fffffffe1b8) at 
../../../git/gcc/toplev.c:2435
#22 0x0000000000593d8c in main (argc=12, argv=0x7fffffffe1b8) at 
../../../git/gcc/main.c:36
(gdb) l
87	  struct c_binding * xlimit = x;
88	  while (ggc_test_and_set_mark (xlimit))
89	   xlimit = ((*xlimit).prev);
90	  while (x != xlimit)
91	    {
92	      switch (TREE_CODE ((*x).decl) == LABEL_DECL)
93	        {
94	        case 0:
95	          gt_ggc_m_9tree_node ((*x).u.type);
96	          break;
(gdb) p *x
$1 = {u = {type = 0x0, label = 0x0}, decl = 0x0, id = 0x0, prev = 
0x7ffff6f77210, shadowed = 0x0, depth = 0, invisible = 0, nested = 0, 
inner_comp = 0, in_struct = 0, locus = 0}
(gdb)


-- 
Jie Zhang
CodeSourcery

Re: [PATCH][C] Always use TRANSLATION_UNIT_DECL as context

[Richard Guenther]

On Mon, 27 Sep 2010, Jie Zhang wrote:

> On 09/27/2010 05:25 PM, Richard Guenther wrote:
> > so I guess we should instead try to not push builtins to the global
> > scope.  Like with
> > 
> > Index: c-decl.c
> > ===================================================================
> > --- c-decl.c    (revision 164591)
> > +++ c-decl.c    (working copy)
> > @@ -1142,6 +1142,10 @@ pop_scope (void)
> >                           "inline function %q+D declared but never
> > defined", p);
> >                DECL_EXTERNAL (p) = 1;
> >              }
> > +         else if (TREE_CODE (p) == FUNCTION_DECL
> > +&&  DECL_BUILT_IN (p))
> > +           /* Do not put builtin functions into the outermost BLOCK.  */
> > +           continue;
> > 
> >            goto common_symbol;
> > 
> > 
> > Joseph might have an idea here (I also see external_scope being used,
> > but I'm not sure what functions actually end up in that).
> > 
> > Does the above help?
> > 
> This change fixed the original regression. But it caused some new ICEs. Like
> 
> Run cc1 under GDB:

Hm, I will later look into this.  Btw, as arm has vectorizing intrinsics
it should better implement targetm.builtin_decl - that there is no
LTO testcase using those builtins is no execuse for it to not do that ;)

Richard.

> (gdb) set args -fpreprocessed memcpy-chk-lib.i -quiet -dumpbase
> memcpy-chk-lib.c -auxbase memcpy-chk-lib -O3 -fomit-frame-pointer -o
> memcpy-chk-lib.s
> (gdb) r
> Starting program: /home/jie/sources/gcc/builds/build-arm-eabi.git/gcc/cc1
> -fpreprocessed memcpy-chk-lib.i -quiet -dumpbase memcpy-chk-lib.c -auxbase
> memcpy-chk-lib -O3 -fomit-frame-pointer -o memcpy-chk-lib.s
> 
> Program received signal SIGSEGV, Segmentation fault.
> 0x00000000004c0931 in gt_ggc_mx_c_binding (x_p=0x7ffff6f771e0) at
> ./gt-c-decl.h:92
> (gdb) bt
> #0  0x00000000004c0931 in gt_ggc_mx_c_binding (x_p=0x7ffff6f771e0) at
> ./gt-c-decl.h:92
> #1  0x00000000004c3027 in gt_ggc_mx_lang_tree_node (x_p=0x7ffff6f40b90) at
> ./gt-c-decl.h:561
> #2  0x00000000004c215f in gt_ggc_mx_lang_tree_node (x_p=0x7ffff6f43700) at
> ./gt-c-decl.h:370
> #3  0x000000000084b4ff in gt_ggc_mx_cgraph_node (x_p=0x7ffff6f92000) at
> gtype-desc.c:524
> #4  0x0000000000851920 in gt_ggc_m_P11cgraph_node4htab (x_p=0x7ffff6e9e930) at
> gtype-desc.c:2432
> #5  0x00000000007e722f in ggc_mark_root_tab (rt=0x11afce0) at
> ../../../git/gcc/ggc-common.c:159
> #6  0x00000000007e72cb in ggc_mark_roots () at
> ../../../git/gcc/ggc-common.c:178
> #7  0x000000000059c45f in ggc_collect () at ../../../git/gcc/ggc-page.c:1950
> #8  0x00000000008f74b9 in execute_todo (flags=2083) at
> ../../../git/gcc/passes.c:1306
> #9  0x00000000008f7cef in execute_one_pass (pass=0x166c180) at
> ../../../git/gcc/passes.c:1592
> #10 0x00000000008f7e1b in execute_pass_list (pass=0x166c180) at
> ../../../git/gcc/passes.c:1624
> #11 0x00000000008f7e3c in execute_pass_list (pass=0x166bfa0) at
> ../../../git/gcc/passes.c:1625
> #12 0x00000000008f7e3c in execute_pass_list (pass=0x166b640) at
> ../../../git/gcc/passes.c:1625
> #13 0x0000000000a7e6e4 in tree_rest_of_compilation (fndecl=0x7ffff6f2af00) at
> ../../../git/gcc/tree-optimize.c:419
> #14 0x0000000000cf1413 in cgraph_expand_function (node=0x7ffff6d952c0) at
> ../../../git/gcc/cgraphunit.c:1477
> #15 0x0000000000cf16ab in cgraph_expand_all_functions () at
> ../../../git/gcc/cgraphunit.c:1556
> #16 0x0000000000cf1cd0 in cgraph_optimize () at
> ../../../git/gcc/cgraphunit.c:1812
> #17 0x0000000000cef817 in cgraph_finalize_compilation_unit () at
> ../../../git/gcc/cgraphunit.c:1020
> #18 0x00000000004c044c in c_write_global_declarations () at
> ../../../git/gcc/c-decl.c:9748
> #19 0x00000000009eb15f in compile_file () at ../../../git/gcc/toplev.c:967
> #20 0x00000000009ed64a in do_compile () at ../../../git/gcc/toplev.c:2394
> #21 0x00000000009ed718 in toplev_main (argc=12, argv=0x7fffffffe1b8) at
> ../../../git/gcc/toplev.c:2435
> #22 0x0000000000593d8c in main (argc=12, argv=0x7fffffffe1b8) at
> ../../../git/gcc/main.c:36
> (gdb) l
> 87	  struct c_binding * xlimit = x;
> 88	  while (ggc_test_and_set_mark (xlimit))
> 89	   xlimit = ((*xlimit).prev);
> 90	  while (x != xlimit)
> 91	    {
> 92	      switch (TREE_CODE ((*x).decl) == LABEL_DECL)
> 93	        {
> 94	        case 0:
> 95	          gt_ggc_m_9tree_node ((*x).u.type);
> 96	          break;
> (gdb) p *x
> $1 = {u = {type = 0x0, label = 0x0}, decl = 0x0, id = 0x0, prev =
> 0x7ffff6f77210, shadowed = 0x0, depth = 0, invisible = 0, nested = 0,
> inner_comp = 0, in_struct = 0, locus = 0}
> (gdb)
> 
> 
> 

-- 
Richard Guenther <rguenther@suse.de>
Novell / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 - GF: Markus Rex

Re: [PATCH][C] Always use TRANSLATION_UNIT_DECL as context

[Richard Guenther]

On Mon, 27 Sep 2010, Richard Guenther wrote:

> On Mon, 27 Sep 2010, Jie Zhang wrote:
> 
> > On 09/27/2010 05:25 PM, Richard Guenther wrote:
> > > so I guess we should instead try to not push builtins to the global
> > > scope.  Like with
> > > 
> > > Index: c-decl.c
> > > ===================================================================
> > > --- c-decl.c    (revision 164591)
> > > +++ c-decl.c    (working copy)
> > > @@ -1142,6 +1142,10 @@ pop_scope (void)
> > >                           "inline function %q+D declared but never
> > > defined", p);
> > >                DECL_EXTERNAL (p) = 1;
> > >              }
> > > +         else if (TREE_CODE (p) == FUNCTION_DECL
> > > +&&  DECL_BUILT_IN (p))
> > > +           /* Do not put builtin functions into the outermost BLOCK.  */
> > > +           continue;
> > > 
> > >            goto common_symbol;
> > > 
> > > 
> > > Joseph might have an idea here (I also see external_scope being used,
> > > but I'm not sure what functions actually end up in that).
> > > 
> > > Does the above help?
> > > 
> > This change fixed the original regression. But it caused some new ICEs. Like
> > 
> > Run cc1 under GDB:
> 
> Hm, I will later look into this.  Btw, as arm has vectorizing intrinsics
> it should better implement targetm.builtin_decl - that there is no
> LTO testcase using those builtins is no execuse for it to not do that ;)

It works on x86-64 if I comment its builtin_decl support with the
single expected FAIL of gcc.dg/lto/20090206-1_0.c
(which relies on proper target builtin streaming).

So I think this is ineed an issue of the backend not properly keeping
the builtins life if they are not used.

Richard.

> Richard.
> 
> > (gdb) set args -fpreprocessed memcpy-chk-lib.i -quiet -dumpbase
> > memcpy-chk-lib.c -auxbase memcpy-chk-lib -O3 -fomit-frame-pointer -o
> > memcpy-chk-lib.s
> > (gdb) r
> > Starting program: /home/jie/sources/gcc/builds/build-arm-eabi.git/gcc/cc1
> > -fpreprocessed memcpy-chk-lib.i -quiet -dumpbase memcpy-chk-lib.c -auxbase
> > memcpy-chk-lib -O3 -fomit-frame-pointer -o memcpy-chk-lib.s
> > 
> > Program received signal SIGSEGV, Segmentation fault.
> > 0x00000000004c0931 in gt_ggc_mx_c_binding (x_p=0x7ffff6f771e0) at
> > ./gt-c-decl.h:92
> > (gdb) bt
> > #0  0x00000000004c0931 in gt_ggc_mx_c_binding (x_p=0x7ffff6f771e0) at
> > ./gt-c-decl.h:92
> > #1  0x00000000004c3027 in gt_ggc_mx_lang_tree_node (x_p=0x7ffff6f40b90) at
> > ./gt-c-decl.h:561
> > #2  0x00000000004c215f in gt_ggc_mx_lang_tree_node (x_p=0x7ffff6f43700) at
> > ./gt-c-decl.h:370
> > #3  0x000000000084b4ff in gt_ggc_mx_cgraph_node (x_p=0x7ffff6f92000) at
> > gtype-desc.c:524
> > #4  0x0000000000851920 in gt_ggc_m_P11cgraph_node4htab (x_p=0x7ffff6e9e930) at
> > gtype-desc.c:2432
> > #5  0x00000000007e722f in ggc_mark_root_tab (rt=0x11afce0) at
> > ../../../git/gcc/ggc-common.c:159
> > #6  0x00000000007e72cb in ggc_mark_roots () at
> > ../../../git/gcc/ggc-common.c:178
> > #7  0x000000000059c45f in ggc_collect () at ../../../git/gcc/ggc-page.c:1950
> > #8  0x00000000008f74b9 in execute_todo (flags=2083) at
> > ../../../git/gcc/passes.c:1306
> > #9  0x00000000008f7cef in execute_one_pass (pass=0x166c180) at
> > ../../../git/gcc/passes.c:1592
> > #10 0x00000000008f7e1b in execute_pass_list (pass=0x166c180) at
> > ../../../git/gcc/passes.c:1624
> > #11 0x00000000008f7e3c in execute_pass_list (pass=0x166bfa0) at
> > ../../../git/gcc/passes.c:1625
> > #12 0x00000000008f7e3c in execute_pass_list (pass=0x166b640) at
> > ../../../git/gcc/passes.c:1625
> > #13 0x0000000000a7e6e4 in tree_rest_of_compilation (fndecl=0x7ffff6f2af00) at
> > ../../../git/gcc/tree-optimize.c:419
> > #14 0x0000000000cf1413 in cgraph_expand_function (node=0x7ffff6d952c0) at
> > ../../../git/gcc/cgraphunit.c:1477
> > #15 0x0000000000cf16ab in cgraph_expand_all_functions () at
> > ../../../git/gcc/cgraphunit.c:1556
> > #16 0x0000000000cf1cd0 in cgraph_optimize () at
> > ../../../git/gcc/cgraphunit.c:1812
> > #17 0x0000000000cef817 in cgraph_finalize_compilation_unit () at
> > ../../../git/gcc/cgraphunit.c:1020
> > #18 0x00000000004c044c in c_write_global_declarations () at
> > ../../../git/gcc/c-decl.c:9748
> > #19 0x00000000009eb15f in compile_file () at ../../../git/gcc/toplev.c:967
> > #20 0x00000000009ed64a in do_compile () at ../../../git/gcc/toplev.c:2394
> > #21 0x00000000009ed718 in toplev_main (argc=12, argv=0x7fffffffe1b8) at
> > ../../../git/gcc/toplev.c:2435
> > #22 0x0000000000593d8c in main (argc=12, argv=0x7fffffffe1b8) at
> > ../../../git/gcc/main.c:36
> > (gdb) l
> > 87	  struct c_binding * xlimit = x;
> > 88	  while (ggc_test_and_set_mark (xlimit))
> > 89	   xlimit = ((*xlimit).prev);
> > 90	  while (x != xlimit)
> > 91	    {
> > 92	      switch (TREE_CODE ((*x).decl) == LABEL_DECL)
> > 93	        {
> > 94	        case 0:
> > 95	          gt_ggc_m_9tree_node ((*x).u.type);
> > 96	          break;
> > (gdb) p *x
> > $1 = {u = {type = 0x0, label = 0x0}, decl = 0x0, id = 0x0, prev =
> > 0x7ffff6f77210, shadowed = 0x0, depth = 0, invisible = 0, nested = 0,
> > inner_comp = 0, in_struct = 0, locus = 0}
> > (gdb)
> > 
> > 
> > 
> 
> 

-- 
Richard Guenther <rguenther@suse.de>
Novell / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 - GF: Markus Rex

Re: [PATCH][C] Always use TRANSLATION_UNIT_DECL as context

[Steve Ellcey]

FYI: I am getting the gimple bytecode messages on my IA64 Linux and
HP-UX platforms as well when running the testsuite.

/proj/opensrc_nobackup/nightly2/src/trunk/gcc/testsuite/gcc.c-torture/compile/20000105-1.c:25:1: sorry, unimplemented: gimple bytecode streams do not support machine specific builtin functions on this target

Steve Ellcey
sje@cup.hp.com

Re: [PATCH][C] Always use TRANSLATION_UNIT_DECL as context

[Richard Guenther]

On Mon, 27 Sep 2010, Steve Ellcey wrote:

> FYI: I am getting the gimple bytecode messages on my IA64 Linux and
> HP-UX platforms as well when running the testsuite.
> 
> /proj/opensrc_nobackup/nightly2/src/trunk/gcc/testsuite/gcc.c-torture/compile/20000105-1.c:25:1: sorry, unimplemented: gimple bytecode streams do not support machine specific builtin functions on this target

Yep, that's a known issue now.  Really all targets with builtins
need to implement targetm.builtin_decl for LTO to work for them.

Richard.

Re: [PATCH][C] Always use TRANSLATION_UNIT_DECL as context

[Steve Ellcey]

[-- Attachment #1: Type: text/plain, Size: 706 bytes --]

On Tue, 2010-09-28 at 10:36 +0200, Richard Guenther wrote:
> On Mon, 27 Sep 2010, Steve Ellcey wrote:
> 
> > FYI: I am getting the gimple bytecode messages on my IA64 Linux and
> > HP-UX platforms as well when running the testsuite.
> > 
> > /proj/opensrc_nobackup/nightly2/src/trunk/gcc/testsuite/gcc.c-torture/compile/20000105-1.c:25:1: sorry, unimplemented: gimple bytecode streams do not support machine specific builtin functions on this target
> 
> Yep, that's a known issue now.  Really all targets with builtins
> need to implement targetm.builtin_decl for LTO to work for them.
> 
> Richard.

OK, I am testing the attached patch for IA64 to fix the builtin problem.

Steve Ellcey
sje@cup.hp.com



[-- Attachment #2: diff.out --]
[-- Type: text/x-patch, Size: 4006 bytes --]

Index: config/ia64/ia64.c
===================================================================
--- config/ia64/ia64.c	(revision 164675)
+++ config/ia64/ia64.c	(working copy)
@@ -319,6 +319,7 @@ static void ia64_trampoline_init (rtx, t
 static void ia64_override_options_after_change (void);
 
 static void ia64_dwarf_handle_frame_unspec (const char *, rtx, int);
+static tree ia64_builtin_decl (unsigned, bool);
 \f
 /* Table of valid machine attributes.  */
 static const struct attribute_spec ia64_attribute_table[] =
@@ -344,6 +345,9 @@ static const struct attribute_spec ia64_
 #undef TARGET_EXPAND_BUILTIN
 #define TARGET_EXPAND_BUILTIN ia64_expand_builtin
 
+#undef TARGET_BUILTIN_DECL
+#define TARGET_BUILTIN_DECL ia64_builtin_decl
+
 #undef TARGET_ASM_BYTE_OP
 #define TARGET_ASM_BYTE_OP "\tdata1\t"
 #undef TARGET_ASM_ALIGNED_HI_OP
@@ -9999,14 +10003,18 @@ enum ia64_builtins
   IA64_BUILTIN_FABSQ,
   IA64_BUILTIN_FLUSHRS,
   IA64_BUILTIN_INFQ,
-  IA64_BUILTIN_HUGE_VALQ
+  IA64_BUILTIN_HUGE_VALQ,
+  IA64_BUILTIN_max
 };
 
+static GTY(()) tree ia64_builtins[(int) IA64_BUILTIN_max];
+
 void
 ia64_init_builtins (void)
 {
   tree fpreg_type;
   tree float80_type;
+  tree decl;
 
   /* The __fpreg type.  */
   fpreg_type = make_node (REAL_TYPE);
@@ -10023,7 +10031,7 @@ ia64_init_builtins (void)
   /* The __float128 type.  */
   if (!TARGET_HPUX)
     {
-      tree ftype, decl;
+      tree ftype;
       tree float128_type = make_node (REAL_TYPE);
 
       TYPE_PRECISION (float128_type) = 128;
@@ -10032,13 +10040,15 @@ ia64_init_builtins (void)
 
       /* TFmode support builtins.  */
       ftype = build_function_type (float128_type, void_list_node);
-      add_builtin_function ("__builtin_infq", ftype,
-			    IA64_BUILTIN_INFQ, BUILT_IN_MD,
-			    NULL, NULL_TREE);
-
-      add_builtin_function ("__builtin_huge_valq", ftype,
-			    IA64_BUILTIN_HUGE_VALQ, BUILT_IN_MD,
-			    NULL, NULL_TREE);
+      decl = add_builtin_function ("__builtin_infq", ftype,
+				   IA64_BUILTIN_INFQ, BUILT_IN_MD,
+				   NULL, NULL_TREE);
+      ia64_builtins[IA64_BUILTIN_INFQ] = decl;
+
+      decl = add_builtin_function ("__builtin_huge_valq", ftype,
+				   IA64_BUILTIN_HUGE_VALQ, BUILT_IN_MD,
+				   NULL, NULL_TREE);
+      ia64_builtins[IA64_BUILTIN_HUGE_VALQ] = decl;
 
       ftype = build_function_type_list (float128_type,
 					float128_type,
@@ -10047,6 +10057,7 @@ ia64_init_builtins (void)
 				   IA64_BUILTIN_FABSQ, BUILT_IN_MD,
 				   "__fabstf2", NULL_TREE);
       TREE_READONLY (decl) = 1;
+      ia64_builtins[IA64_BUILTIN_FABSQ] = decl;
 
       ftype = build_function_type_list (float128_type,
 					float128_type,
@@ -10056,6 +10067,7 @@ ia64_init_builtins (void)
 				   IA64_BUILTIN_COPYSIGNQ, BUILT_IN_MD,
 				   "__copysigntf3", NULL_TREE);
       TREE_READONLY (decl) = 1;
+      ia64_builtins[IA64_BUILTIN_COPYSIGNQ] = decl;
     }
   else
     /* Under HPUX, this is a synonym for "long double".  */
@@ -10073,13 +10085,15 @@ ia64_init_builtins (void)
   add_builtin_function ((name), (type), (code), BUILT_IN_MD,	\
 		       NULL, NULL_TREE)
 
-  def_builtin ("__builtin_ia64_bsp",
+  decl = def_builtin ("__builtin_ia64_bsp",
 	       build_function_type (ptr_type_node, void_list_node),
 	       IA64_BUILTIN_BSP);
+  ia64_builtins[IA64_BUILTIN_BSP] = decl;
 
-  def_builtin ("__builtin_ia64_flushrs",
+  decl = def_builtin ("__builtin_ia64_flushrs",
 	       build_function_type (void_type_node, void_list_node),
 	       IA64_BUILTIN_FLUSHRS);
+  ia64_builtins[IA64_BUILTIN_FLUSHRS] = decl;
 
 #undef def_builtin
 
@@ -10149,6 +10163,17 @@ ia64_expand_builtin (tree exp, rtx targe
   return NULL_RTX;
 }
 
+/* Return the ia64 builtin for CODE.  */
+
+static tree
+ia64_builtin_decl (unsigned code, bool initialize_p ATTRIBUTE_UNUSED)
+{
+  if (code >= IA64_BUILTIN_max)
+    return error_mark_node;
+
+  return ia64_builtins[code];
+}
+
 /* For the HP-UX IA64 aggregate parameters are passed stored in the
    most significant bits of the stack slot.  */
 

Re: [PATCH][C] Always use TRANSLATION_UNIT_DECL as context

[Richard Guenther]

On Tue, 28 Sep 2010, Steve Ellcey wrote:

> On Tue, 2010-09-28 at 10:36 +0200, Richard Guenther wrote:
> > On Mon, 27 Sep 2010, Steve Ellcey wrote:
> > 
> > > FYI: I am getting the gimple bytecode messages on my IA64 Linux and
> > > HP-UX platforms as well when running the testsuite.
> > > 
> > > /proj/opensrc_nobackup/nightly2/src/trunk/gcc/testsuite/gcc.c-torture/compile/20000105-1.c:25:1: sorry, unimplemented: gimple bytecode streams do not support machine specific builtin functions on this target
> > 
> > Yep, that's a known issue now.  Really all targets with builtins
> > need to implement targetm.builtin_decl for LTO to work for them.
> > 
> > Richard.
> 
> OK, I am testing the attached patch for IA64 to fix the builtin problem.

Thanks.  FYI I'm still poking at a solution that would avoid
the situation but I'll let folks some time to update their targets
to implememt that hook (we're in stage1 anyways).

Richard.