* [PATCH] Limit alignment on error_mark_node variable @ 2015-07-08 15:32 H.J. Lu 2015-07-09 8:16 ` Richard Biener 0 siblings, 1 reply; 10+ messages in thread From: H.J. Lu @ 2015-07-08 15:32 UTC (permalink / raw) To: gcc-patches There is no need to try different alignment on variable of error_mark_node. OK for trunk if there is no regression? Thanks. H.J. -- gcc/ PR target/66810 * varasm.c (align_variable): Don't try different alignment on variable of error_mark_node. gcc/testsuite/ PR target/66810 * gcc.target/i386/pr66810.c: New test. --- gcc/testsuite/gcc.target/i386/pr66810.c | 10 ++++++++++ gcc/varasm.c | 3 ++- 2 files changed, 12 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gcc.target/i386/pr66810.c diff --git a/gcc/testsuite/gcc.target/i386/pr66810.c b/gcc/testsuite/gcc.target/i386/pr66810.c new file mode 100644 index 0000000..4778b37 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr66810.c @@ -0,0 +1,10 @@ +/* { dg-do compile { target ia32 } } */ +/* { dg-options "-mno-sse -mno-mmx -miamcu" } */ + +int vv; + +void +i (void) +{ + static int a[vv]; /* { dg-error "storage size" } */ +} diff --git a/gcc/varasm.c b/gcc/varasm.c index b69b3a3..be33cb4 100644 --- a/gcc/varasm.c +++ b/gcc/varasm.c @@ -1016,7 +1016,8 @@ align_variable (tree decl, bool dont_output_data) align = MAX_OFILE_ALIGNMENT; } - if (! DECL_USER_ALIGN (decl)) + /* Don't try different alignment for error_mark_node. */ + if (! DECL_USER_ALIGN (decl) && TREE_TYPE (decl) != error_mark_node) { #ifdef DATA_ABI_ALIGNMENT unsigned int data_abi_align -- 2.4.3 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Limit alignment on error_mark_node variable 2015-07-08 15:32 [PATCH] Limit alignment on error_mark_node variable H.J. Lu @ 2015-07-09 8:16 ` Richard Biener 2015-07-09 9:52 ` H.J. Lu 0 siblings, 1 reply; 10+ messages in thread From: Richard Biener @ 2015-07-09 8:16 UTC (permalink / raw) To: H.J. Lu; +Cc: GCC Patches On Wed, Jul 8, 2015 at 5:32 PM, H.J. Lu <hongjiu.lu@intel.com> wrote: > There is no need to try different alignment on variable of > error_mark_node. > > OK for trunk if there is no regression? Can't we avoid calling align_variable on error_mark_node type decls completely? That is, punt earlier when we try to emit it. > Thanks. > > H.J. > -- > gcc/ > > PR target/66810 > * varasm.c (align_variable): Don't try different alignment on > variable of error_mark_node. > > gcc/testsuite/ > > PR target/66810 > * gcc.target/i386/pr66810.c: New test. > --- > gcc/testsuite/gcc.target/i386/pr66810.c | 10 ++++++++++ > gcc/varasm.c | 3 ++- > 2 files changed, 12 insertions(+), 1 deletion(-) > create mode 100644 gcc/testsuite/gcc.target/i386/pr66810.c > > diff --git a/gcc/testsuite/gcc.target/i386/pr66810.c b/gcc/testsuite/gcc.target/i386/pr66810.c > new file mode 100644 > index 0000000..4778b37 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/i386/pr66810.c > @@ -0,0 +1,10 @@ > +/* { dg-do compile { target ia32 } } */ > +/* { dg-options "-mno-sse -mno-mmx -miamcu" } */ > + > +int vv; > + > +void > +i (void) > +{ > + static int a[vv]; /* { dg-error "storage size" } */ > +} > diff --git a/gcc/varasm.c b/gcc/varasm.c > index b69b3a3..be33cb4 100644 > --- a/gcc/varasm.c > +++ b/gcc/varasm.c > @@ -1016,7 +1016,8 @@ align_variable (tree decl, bool dont_output_data) > align = MAX_OFILE_ALIGNMENT; > } > > - if (! DECL_USER_ALIGN (decl)) > + /* Don't try different alignment for error_mark_node. */ > + if (! DECL_USER_ALIGN (decl) && TREE_TYPE (decl) != error_mark_node) > { > #ifdef DATA_ABI_ALIGNMENT > unsigned int data_abi_align > -- > 2.4.3 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Limit alignment on error_mark_node variable 2015-07-09 8:16 ` Richard Biener @ 2015-07-09 9:52 ` H.J. Lu 2015-07-09 9:54 ` Richard Biener 0 siblings, 1 reply; 10+ messages in thread From: H.J. Lu @ 2015-07-09 9:52 UTC (permalink / raw) To: Richard Biener; +Cc: GCC Patches On Thu, Jul 09, 2015 at 10:16:38AM +0200, Richard Biener wrote: > On Wed, Jul 8, 2015 at 5:32 PM, H.J. Lu <hongjiu.lu@intel.com> wrote: > > There is no need to try different alignment on variable of > > error_mark_node. > > > > OK for trunk if there is no regression? > > Can't we avoid calling align_variable on error_mark_node type decls > completely? That is, punt earlier when we try to emit it. > How about this? OK for trunk? H.J. --- There is no need to analyze error_mark_node type decls. gcc/ PR target/66810 * varpool.cvarpool.c (varpool_node::analyze): Skip error_mark_node type decls. gcc/testsuite/ PR target/66810 * gcc.target/i386/pr66810.c: New test. --- gcc/testsuite/gcc.target/i386/pr66810.c | 10 ++++++++++ gcc/varpool.c | 29 +++++++++++++++++------------ 2 files changed, 27 insertions(+), 12 deletions(-) create mode 100644 gcc/testsuite/gcc.target/i386/pr66810.c diff --git a/gcc/testsuite/gcc.target/i386/pr66810.c b/gcc/testsuite/gcc.target/i386/pr66810.c new file mode 100644 index 0000000..4778b37 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr66810.c @@ -0,0 +1,10 @@ +/* { dg-do compile { target ia32 } } */ +/* { dg-options "-mno-sse -mno-mmx -miamcu" } */ + +int vv; + +void +i (void) +{ + static int a[vv]; /* { dg-error "storage size" } */ +} diff --git a/gcc/varpool.c b/gcc/varpool.c index 10fa93c..f7c4d46 100644 --- a/gcc/varpool.c +++ b/gcc/varpool.c @@ -514,20 +514,25 @@ varpool_node::get_availability (void) void varpool_node::analyze (void) { - /* When reading back varpool at LTO time, we re-construct the queue in order - to have "needed" list right by inserting all needed nodes into varpool. - We however don't want to re-analyze already analyzed nodes. */ - if (!analyzed) + /* Skip error_mark_node type decls. */ + if (TREE_TYPE (decl) != error_mark_node) { - gcc_assert (!in_lto_p || symtab->function_flags_ready); - /* Compute the alignment early so function body expanders are - already informed about increased alignment. */ - align_variable (decl, 0); + /* When reading back varpool at LTO time, we re-construct the + queue in order to have "needed" list right by inserting all + needed nodes into varpool. We however don't want to re-analyze + already analyzed nodes. */ + if (!analyzed) + { + gcc_assert (!in_lto_p || symtab->function_flags_ready); + /* Compute the alignment early so function body expanders are + already informed about increased alignment. */ + align_variable (decl, 0); + } + if (alias) + resolve_alias (varpool_node::get (alias_target)); + else if (DECL_INITIAL (decl)) + record_references_in_initializer (decl, analyzed); } - if (alias) - resolve_alias (varpool_node::get (alias_target)); - else if (DECL_INITIAL (decl)) - record_references_in_initializer (decl, analyzed); analyzed = true; } -- 2.4.3 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Limit alignment on error_mark_node variable 2015-07-09 9:52 ` H.J. Lu @ 2015-07-09 9:54 ` Richard Biener 2015-07-09 11:08 ` H.J. Lu 0 siblings, 1 reply; 10+ messages in thread From: Richard Biener @ 2015-07-09 9:54 UTC (permalink / raw) To: H.J. Lu; +Cc: GCC Patches On Thu, Jul 9, 2015 at 11:52 AM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Thu, Jul 09, 2015 at 10:16:38AM +0200, Richard Biener wrote: >> On Wed, Jul 8, 2015 at 5:32 PM, H.J. Lu <hongjiu.lu@intel.com> wrote: >> > There is no need to try different alignment on variable of >> > error_mark_node. >> > >> > OK for trunk if there is no regression? >> >> Can't we avoid calling align_variable on error_mark_node type decls >> completely? That is, punt earlier when we try to emit it. >> > > How about this? OK for trunk? Heh, you now get the obvious question why we can't simply avoid adding the varpool node in the first place ;) Richard. > H.J. > --- > There is no need to analyze error_mark_node type decls. > > gcc/ > > PR target/66810 > * varpool.cvarpool.c (varpool_node::analyze): Skip > error_mark_node type decls. > > gcc/testsuite/ > > PR target/66810 > * gcc.target/i386/pr66810.c: New test. > --- > gcc/testsuite/gcc.target/i386/pr66810.c | 10 ++++++++++ > gcc/varpool.c | 29 +++++++++++++++++------------ > 2 files changed, 27 insertions(+), 12 deletions(-) > create mode 100644 gcc/testsuite/gcc.target/i386/pr66810.c > > diff --git a/gcc/testsuite/gcc.target/i386/pr66810.c b/gcc/testsuite/gcc.target/i386/pr66810.c > new file mode 100644 > index 0000000..4778b37 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/i386/pr66810.c > @@ -0,0 +1,10 @@ > +/* { dg-do compile { target ia32 } } */ > +/* { dg-options "-mno-sse -mno-mmx -miamcu" } */ > + > +int vv; > + > +void > +i (void) > +{ > + static int a[vv]; /* { dg-error "storage size" } */ > +} > diff --git a/gcc/varpool.c b/gcc/varpool.c > index 10fa93c..f7c4d46 100644 > --- a/gcc/varpool.c > +++ b/gcc/varpool.c > @@ -514,20 +514,25 @@ varpool_node::get_availability (void) > void > varpool_node::analyze (void) > { > - /* When reading back varpool at LTO time, we re-construct the queue in order > - to have "needed" list right by inserting all needed nodes into varpool. > - We however don't want to re-analyze already analyzed nodes. */ > - if (!analyzed) > + /* Skip error_mark_node type decls. */ > + if (TREE_TYPE (decl) != error_mark_node) > { > - gcc_assert (!in_lto_p || symtab->function_flags_ready); > - /* Compute the alignment early so function body expanders are > - already informed about increased alignment. */ > - align_variable (decl, 0); > + /* When reading back varpool at LTO time, we re-construct the > + queue in order to have "needed" list right by inserting all > + needed nodes into varpool. We however don't want to re-analyze > + already analyzed nodes. */ > + if (!analyzed) > + { > + gcc_assert (!in_lto_p || symtab->function_flags_ready); > + /* Compute the alignment early so function body expanders are > + already informed about increased alignment. */ > + align_variable (decl, 0); > + } > + if (alias) > + resolve_alias (varpool_node::get (alias_target)); > + else if (DECL_INITIAL (decl)) > + record_references_in_initializer (decl, analyzed); > } > - if (alias) > - resolve_alias (varpool_node::get (alias_target)); > - else if (DECL_INITIAL (decl)) > - record_references_in_initializer (decl, analyzed); > analyzed = true; > } > > -- > 2.4.3 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Limit alignment on error_mark_node variable 2015-07-09 9:54 ` Richard Biener @ 2015-07-09 11:08 ` H.J. Lu 2015-07-09 13:57 ` Richard Biener 0 siblings, 1 reply; 10+ messages in thread From: H.J. Lu @ 2015-07-09 11:08 UTC (permalink / raw) To: Richard Biener; +Cc: GCC Patches On Thu, Jul 9, 2015 at 2:54 AM, Richard Biener <richard.guenther@gmail.com> wrote: > On Thu, Jul 9, 2015 at 11:52 AM, H.J. Lu <hjl.tools@gmail.com> wrote: >> On Thu, Jul 09, 2015 at 10:16:38AM +0200, Richard Biener wrote: >>> On Wed, Jul 8, 2015 at 5:32 PM, H.J. Lu <hongjiu.lu@intel.com> wrote: >>> > There is no need to try different alignment on variable of >>> > error_mark_node. >>> > >>> > OK for trunk if there is no regression? >>> >>> Can't we avoid calling align_variable on error_mark_node type decls >>> completely? That is, punt earlier when we try to emit it. >>> >> >> How about this? OK for trunk? > > Heh, you now get the obvious question why we can't simply avoid > adding the varpool node in the first place ;) > When it was first added to varpool, its type was OK: (gdb) bt #0 varpool_node::get_create (decl=<var_decl 0x7ffff1506900 vv>) at /export/gnu/import/git/sources/gcc/gcc/varpool.c:150 #1 0x0000000000e1c3e8 in rest_of_decl_compilation ( decl=<var_decl 0x7ffff1506900 vv>, top_level=1, at_end=0) at /export/gnu/import/git/sources/gcc/gcc/passes.c:271 #2 0x0000000000731d39 in finish_decl (decl=<var_decl 0x7ffff1506900 vv>, init_loc=0, init=<tree 0x0>, origtype=<tree 0x0>, asmspec_tree=<tree 0x0>) at /export/gnu/import/git/sources/gcc/gcc/c/c-decl.c:4863 #3 0x000000000078d1ed in c_parser_declaration_or_fndef ( parser=0x7ffff15050a8, fndef_ok=false, static_assert_ok=true, empty_ok=true, nested=false, start_attr_ok=true, objc_foreach_object_declaration=0x0, omp_declare_simd_clauses=...) at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:1855 #4 0x000000000078c234 in c_parser_external_declaration (parser=0x7ffff15050a8) at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:1435 #5 0x000000000078be45 in c_parser_translation_unit (parser=0x7ffff15050a8) at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:1322 #6 0x00000000007b3271 in c_parse_file () at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:15440 #7 0x000000000081cb97 in c_common_parse_file () at /export/gnu/import/git/sources/gcc/gcc/c-family/c-opts.c:1059 #8 0x0000000000f27662 in compile_file () at /export/gnu/import/git/sources/gcc/gcc/toplev.c:543 ---Type <return> to continue, or q <return> to quit--- #9 0x0000000000f29baa in do_compile () at /export/gnu/import/git/sources/gcc/gcc/toplev.c:2041 #10 0x0000000000f29df9 in toplev::main (this=0x7fffffffdc90, argc=17, argv=0x7fffffffdd98) at /export/gnu/import/git/sources/gcc/gcc/toplev.c:2142 #11 0x00000000017d8228 in main (argc=17, argv=0x7fffffffdd98) at /export/gnu/import/git/sources/gcc/gcc/main.c:39 Later, it was turned into error_mark_node: Old value = <array_type 0x7ffff15ee150> New value = <error_mark 0x7ffff14fac90> finish_decl (decl=<var_decl 0x7ffff1506990 a>, init_loc=0, init=<tree 0x0>, origtype=<tree 0x0>, asmspec_tree=<tree 0x0>) at /export/gnu/import/git/sources/gcc/gcc/c/c-decl.c:4802 4802 if (TREE_USED (type)) (gdb) bt #0 finish_decl (decl=<var_decl 0x7ffff1506990 a>, init_loc=0, init=<tree 0x0>, origtype=<tree 0x0>, asmspec_tree=<tree 0x0>) at /export/gnu/import/git/sources/gcc/gcc/c/c-decl.c:4802 #1 0x000000000078d1ed in c_parser_declaration_or_fndef ( parser=0x7ffff15050a8, fndef_ok=false, static_assert_ok=true, empty_ok=true, nested=true, start_attr_ok=true, objc_foreach_object_declaration=0x0, omp_declare_simd_clauses=...) at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:1855 #2 0x0000000000792a23 in c_parser_compound_statement_nostart ( parser=0x7ffff15050a8) at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:4621 #3 0x0000000000792688 in c_parser_compound_statement (parser=0x7ffff15050a8) at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:4532 #4 0x000000000078d5a3 in c_parser_declaration_or_fndef ( parser=0x7ffff15050a8, fndef_ok=true, static_assert_ok=true, empty_ok=true, nested=false, start_attr_ok=true, objc_foreach_object_declaration=0x0, omp_declare_simd_clauses=...) at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:1965 #5 0x000000000078c234 in c_parser_external_declaration (parser=0x7ffff15050a8) at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:1435 #6 0x000000000078be45 in c_parser_translation_unit (parser=0x7ffff15050a8) at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:1322 #7 0x00000000007b3271 in c_parse_file () ---Type <return> to continue, or q <return> to quit--- at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:15440 #8 0x000000000081cb97 in c_common_parse_file () at /export/gnu/import/git/sources/gcc/gcc/c-family/c-opts.c:1059 #9 0x0000000000f27662 in compile_file () at /export/gnu/import/git/sources/gcc/gcc/toplev.c:543 #10 0x0000000000f29baa in do_compile () at /export/gnu/import/git/sources/gcc/gcc/toplev.c:2041 #11 0x0000000000f29df9 in toplev::main (this=0x7fffffffdc90, argc=17, argv=0x7fffffffdd98) at /export/gnu/import/git/sources/gcc/gcc/toplev.c:2142 #12 0x00000000017d8228 in main (argc=17, argv=0x7fffffffdd98) at /export/gnu/import/git/sources/gcc/gcc/main.c:39 (gdb) I guess it isn't worth to take it out of varpool. Is my patch OK for trunk? -- H.J. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Limit alignment on error_mark_node variable 2015-07-09 11:08 ` H.J. Lu @ 2015-07-09 13:57 ` Richard Biener 2015-07-09 14:06 ` H.J. Lu 2015-07-10 12:19 ` H.J. Lu 0 siblings, 2 replies; 10+ messages in thread From: Richard Biener @ 2015-07-09 13:57 UTC (permalink / raw) To: H.J. Lu; +Cc: GCC Patches On Thu, Jul 9, 2015 at 1:08 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Thu, Jul 9, 2015 at 2:54 AM, Richard Biener > <richard.guenther@gmail.com> wrote: >> On Thu, Jul 9, 2015 at 11:52 AM, H.J. Lu <hjl.tools@gmail.com> wrote: >>> On Thu, Jul 09, 2015 at 10:16:38AM +0200, Richard Biener wrote: >>>> On Wed, Jul 8, 2015 at 5:32 PM, H.J. Lu <hongjiu.lu@intel.com> wrote: >>>> > There is no need to try different alignment on variable of >>>> > error_mark_node. >>>> > >>>> > OK for trunk if there is no regression? >>>> >>>> Can't we avoid calling align_variable on error_mark_node type decls >>>> completely? That is, punt earlier when we try to emit it. >>>> >>> >>> How about this? OK for trunk? >> >> Heh, you now get the obvious question why we can't simply avoid >> adding the varpool node in the first place ;) >> > > When it was first added to varpool, its type was OK: > > (gdb) bt > #0 varpool_node::get_create (decl=<var_decl 0x7ffff1506900 vv>) > at /export/gnu/import/git/sources/gcc/gcc/varpool.c:150 > #1 0x0000000000e1c3e8 in rest_of_decl_compilation ( > decl=<var_decl 0x7ffff1506900 vv>, top_level=1, at_end=0) > at /export/gnu/import/git/sources/gcc/gcc/passes.c:271 > #2 0x0000000000731d39 in finish_decl (decl=<var_decl 0x7ffff1506900 vv>, > init_loc=0, init=<tree 0x0>, origtype=<tree 0x0>, asmspec_tree=<tree 0x0>) > at /export/gnu/import/git/sources/gcc/gcc/c/c-decl.c:4863 > #3 0x000000000078d1ed in c_parser_declaration_or_fndef ( > parser=0x7ffff15050a8, fndef_ok=false, static_assert_ok=true, > empty_ok=true, nested=false, start_attr_ok=true, > objc_foreach_object_declaration=0x0, omp_declare_simd_clauses=...) > at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:1855 > #4 0x000000000078c234 in c_parser_external_declaration (parser=0x7ffff15050a8) > at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:1435 > #5 0x000000000078be45 in c_parser_translation_unit (parser=0x7ffff15050a8) > at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:1322 > #6 0x00000000007b3271 in c_parse_file () > at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:15440 > #7 0x000000000081cb97 in c_common_parse_file () > at /export/gnu/import/git/sources/gcc/gcc/c-family/c-opts.c:1059 > #8 0x0000000000f27662 in compile_file () > at /export/gnu/import/git/sources/gcc/gcc/toplev.c:543 > ---Type <return> to continue, or q <return> to quit--- > #9 0x0000000000f29baa in do_compile () > at /export/gnu/import/git/sources/gcc/gcc/toplev.c:2041 > #10 0x0000000000f29df9 in toplev::main (this=0x7fffffffdc90, argc=17, > argv=0x7fffffffdd98) > at /export/gnu/import/git/sources/gcc/gcc/toplev.c:2142 > #11 0x00000000017d8228 in main (argc=17, argv=0x7fffffffdd98) > at /export/gnu/import/git/sources/gcc/gcc/main.c:39 > > Later, it was turned into error_mark_node: > > Old value = <array_type 0x7ffff15ee150> > New value = <error_mark 0x7ffff14fac90> > finish_decl (decl=<var_decl 0x7ffff1506990 a>, init_loc=0, init=<tree 0x0>, > origtype=<tree 0x0>, asmspec_tree=<tree 0x0>) > at /export/gnu/import/git/sources/gcc/gcc/c/c-decl.c:4802 > 4802 if (TREE_USED (type)) > (gdb) bt > #0 finish_decl (decl=<var_decl 0x7ffff1506990 a>, init_loc=0, > init=<tree 0x0>, origtype=<tree 0x0>, asmspec_tree=<tree 0x0>) > at /export/gnu/import/git/sources/gcc/gcc/c/c-decl.c:4802 > #1 0x000000000078d1ed in c_parser_declaration_or_fndef ( > parser=0x7ffff15050a8, fndef_ok=false, static_assert_ok=true, > empty_ok=true, nested=true, start_attr_ok=true, > objc_foreach_object_declaration=0x0, omp_declare_simd_clauses=...) > at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:1855 > #2 0x0000000000792a23 in c_parser_compound_statement_nostart ( > parser=0x7ffff15050a8) > at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:4621 > #3 0x0000000000792688 in c_parser_compound_statement (parser=0x7ffff15050a8) > at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:4532 > #4 0x000000000078d5a3 in c_parser_declaration_or_fndef ( > parser=0x7ffff15050a8, fndef_ok=true, static_assert_ok=true, > empty_ok=true, nested=false, start_attr_ok=true, > objc_foreach_object_declaration=0x0, omp_declare_simd_clauses=...) > at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:1965 > #5 0x000000000078c234 in c_parser_external_declaration (parser=0x7ffff15050a8) > at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:1435 > #6 0x000000000078be45 in c_parser_translation_unit (parser=0x7ffff15050a8) > at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:1322 > #7 0x00000000007b3271 in c_parse_file () > ---Type <return> to continue, or q <return> to quit--- > at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:15440 > #8 0x000000000081cb97 in c_common_parse_file () > at /export/gnu/import/git/sources/gcc/gcc/c-family/c-opts.c:1059 > #9 0x0000000000f27662 in compile_file () > at /export/gnu/import/git/sources/gcc/gcc/toplev.c:543 > #10 0x0000000000f29baa in do_compile () > at /export/gnu/import/git/sources/gcc/gcc/toplev.c:2041 > #11 0x0000000000f29df9 in toplev::main (this=0x7fffffffdc90, argc=17, > argv=0x7fffffffdd98) > at /export/gnu/import/git/sources/gcc/gcc/toplev.c:2142 > #12 0x00000000017d8228 in main (argc=17, argv=0x7fffffffdd98) > at /export/gnu/import/git/sources/gcc/gcc/main.c:39 > (gdb) > > I guess it isn't worth to take it out of varpool. > > Is my patch OK for trunk? I don't see why the C FE would need to invoke finish_decl twice here. So I'd rather have the C frontend not invoke rest_of_decl_compilation on this in the first place. Your patch still feels like a hack in the wrong place. Richard. > -- > H.J. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Limit alignment on error_mark_node variable 2015-07-09 13:57 ` Richard Biener @ 2015-07-09 14:06 ` H.J. Lu 2015-07-09 14:10 ` Richard Biener 2015-07-10 12:19 ` H.J. Lu 1 sibling, 1 reply; 10+ messages in thread From: H.J. Lu @ 2015-07-09 14:06 UTC (permalink / raw) To: Richard Biener; +Cc: GCC Patches On Thu, Jul 9, 2015 at 6:57 AM, Richard Biener <richard.guenther@gmail.com> wrote: > On Thu, Jul 9, 2015 at 1:08 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >> On Thu, Jul 9, 2015 at 2:54 AM, Richard Biener >> <richard.guenther@gmail.com> wrote: >>> On Thu, Jul 9, 2015 at 11:52 AM, H.J. Lu <hjl.tools@gmail.com> wrote: >>>> On Thu, Jul 09, 2015 at 10:16:38AM +0200, Richard Biener wrote: >>>>> On Wed, Jul 8, 2015 at 5:32 PM, H.J. Lu <hongjiu.lu@intel.com> wrote: >>>>> > There is no need to try different alignment on variable of >>>>> > error_mark_node. >>>>> > >>>>> > OK for trunk if there is no regression? >>>>> >>>>> Can't we avoid calling align_variable on error_mark_node type decls >>>>> completely? That is, punt earlier when we try to emit it. >>>>> >>>> >>>> How about this? OK for trunk? >>> >>> Heh, you now get the obvious question why we can't simply avoid >>> adding the varpool node in the first place ;) >>> >> >> When it was first added to varpool, its type was OK: >> >> (gdb) bt >> #0 varpool_node::get_create (decl=<var_decl 0x7ffff1506900 vv>) >> at /export/gnu/import/git/sources/gcc/gcc/varpool.c:150 >> #1 0x0000000000e1c3e8 in rest_of_decl_compilation ( >> decl=<var_decl 0x7ffff1506900 vv>, top_level=1, at_end=0) >> at /export/gnu/import/git/sources/gcc/gcc/passes.c:271 >> #2 0x0000000000731d39 in finish_decl (decl=<var_decl 0x7ffff1506900 vv>, >> init_loc=0, init=<tree 0x0>, origtype=<tree 0x0>, asmspec_tree=<tree 0x0>) >> at /export/gnu/import/git/sources/gcc/gcc/c/c-decl.c:4863 >> #3 0x000000000078d1ed in c_parser_declaration_or_fndef ( >> parser=0x7ffff15050a8, fndef_ok=false, static_assert_ok=true, >> empty_ok=true, nested=false, start_attr_ok=true, >> objc_foreach_object_declaration=0x0, omp_declare_simd_clauses=...) >> at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:1855 >> #4 0x000000000078c234 in c_parser_external_declaration (parser=0x7ffff15050a8) >> at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:1435 >> #5 0x000000000078be45 in c_parser_translation_unit (parser=0x7ffff15050a8) >> at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:1322 >> #6 0x00000000007b3271 in c_parse_file () >> at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:15440 >> #7 0x000000000081cb97 in c_common_parse_file () >> at /export/gnu/import/git/sources/gcc/gcc/c-family/c-opts.c:1059 >> #8 0x0000000000f27662 in compile_file () >> at /export/gnu/import/git/sources/gcc/gcc/toplev.c:543 >> ---Type <return> to continue, or q <return> to quit--- >> #9 0x0000000000f29baa in do_compile () >> at /export/gnu/import/git/sources/gcc/gcc/toplev.c:2041 >> #10 0x0000000000f29df9 in toplev::main (this=0x7fffffffdc90, argc=17, >> argv=0x7fffffffdd98) >> at /export/gnu/import/git/sources/gcc/gcc/toplev.c:2142 >> #11 0x00000000017d8228 in main (argc=17, argv=0x7fffffffdd98) >> at /export/gnu/import/git/sources/gcc/gcc/main.c:39 >> >> Later, it was turned into error_mark_node: >> >> Old value = <array_type 0x7ffff15ee150> >> New value = <error_mark 0x7ffff14fac90> >> finish_decl (decl=<var_decl 0x7ffff1506990 a>, init_loc=0, init=<tree 0x0>, >> origtype=<tree 0x0>, asmspec_tree=<tree 0x0>) >> at /export/gnu/import/git/sources/gcc/gcc/c/c-decl.c:4802 >> 4802 if (TREE_USED (type)) >> (gdb) bt >> #0 finish_decl (decl=<var_decl 0x7ffff1506990 a>, init_loc=0, >> init=<tree 0x0>, origtype=<tree 0x0>, asmspec_tree=<tree 0x0>) >> at /export/gnu/import/git/sources/gcc/gcc/c/c-decl.c:4802 >> #1 0x000000000078d1ed in c_parser_declaration_or_fndef ( >> parser=0x7ffff15050a8, fndef_ok=false, static_assert_ok=true, >> empty_ok=true, nested=true, start_attr_ok=true, >> objc_foreach_object_declaration=0x0, omp_declare_simd_clauses=...) >> at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:1855 >> #2 0x0000000000792a23 in c_parser_compound_statement_nostart ( >> parser=0x7ffff15050a8) >> at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:4621 >> #3 0x0000000000792688 in c_parser_compound_statement (parser=0x7ffff15050a8) >> at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:4532 >> #4 0x000000000078d5a3 in c_parser_declaration_or_fndef ( >> parser=0x7ffff15050a8, fndef_ok=true, static_assert_ok=true, >> empty_ok=true, nested=false, start_attr_ok=true, >> objc_foreach_object_declaration=0x0, omp_declare_simd_clauses=...) >> at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:1965 >> #5 0x000000000078c234 in c_parser_external_declaration (parser=0x7ffff15050a8) >> at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:1435 >> #6 0x000000000078be45 in c_parser_translation_unit (parser=0x7ffff15050a8) >> at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:1322 >> #7 0x00000000007b3271 in c_parse_file () >> ---Type <return> to continue, or q <return> to quit--- >> at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:15440 >> #8 0x000000000081cb97 in c_common_parse_file () >> at /export/gnu/import/git/sources/gcc/gcc/c-family/c-opts.c:1059 >> #9 0x0000000000f27662 in compile_file () >> at /export/gnu/import/git/sources/gcc/gcc/toplev.c:543 >> #10 0x0000000000f29baa in do_compile () >> at /export/gnu/import/git/sources/gcc/gcc/toplev.c:2041 >> #11 0x0000000000f29df9 in toplev::main (this=0x7fffffffdc90, argc=17, >> argv=0x7fffffffdd98) >> at /export/gnu/import/git/sources/gcc/gcc/toplev.c:2142 >> #12 0x00000000017d8228 in main (argc=17, argv=0x7fffffffdd98) >> at /export/gnu/import/git/sources/gcc/gcc/main.c:39 >> (gdb) >> >> I guess it isn't worth to take it out of varpool. >> >> Is my patch OK for trunk? > > I don't see why the C FE would need to invoke finish_decl twice here. > So I'd rather > have the C frontend not invoke rest_of_decl_compilation on this in the > first place. > > Your patch still feels like a hack in the wrong place. > I changed bug report to C and leave it to C FE people. -- H.J. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Limit alignment on error_mark_node variable 2015-07-09 14:06 ` H.J. Lu @ 2015-07-09 14:10 ` Richard Biener 0 siblings, 0 replies; 10+ messages in thread From: Richard Biener @ 2015-07-09 14:10 UTC (permalink / raw) To: H.J. Lu; +Cc: GCC Patches On Thu, Jul 9, 2015 at 4:06 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Thu, Jul 9, 2015 at 6:57 AM, Richard Biener > <richard.guenther@gmail.com> wrote: >> On Thu, Jul 9, 2015 at 1:08 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >>> On Thu, Jul 9, 2015 at 2:54 AM, Richard Biener >>> <richard.guenther@gmail.com> wrote: >>>> On Thu, Jul 9, 2015 at 11:52 AM, H.J. Lu <hjl.tools@gmail.com> wrote: >>>>> On Thu, Jul 09, 2015 at 10:16:38AM +0200, Richard Biener wrote: >>>>>> On Wed, Jul 8, 2015 at 5:32 PM, H.J. Lu <hongjiu.lu@intel.com> wrote: >>>>>> > There is no need to try different alignment on variable of >>>>>> > error_mark_node. >>>>>> > >>>>>> > OK for trunk if there is no regression? >>>>>> >>>>>> Can't we avoid calling align_variable on error_mark_node type decls >>>>>> completely? That is, punt earlier when we try to emit it. >>>>>> >>>>> >>>>> How about this? OK for trunk? >>>> >>>> Heh, you now get the obvious question why we can't simply avoid >>>> adding the varpool node in the first place ;) >>>> >>> >>> When it was first added to varpool, its type was OK: >>> >>> (gdb) bt >>> #0 varpool_node::get_create (decl=<var_decl 0x7ffff1506900 vv>) >>> at /export/gnu/import/git/sources/gcc/gcc/varpool.c:150 >>> #1 0x0000000000e1c3e8 in rest_of_decl_compilation ( >>> decl=<var_decl 0x7ffff1506900 vv>, top_level=1, at_end=0) >>> at /export/gnu/import/git/sources/gcc/gcc/passes.c:271 >>> #2 0x0000000000731d39 in finish_decl (decl=<var_decl 0x7ffff1506900 vv>, >>> init_loc=0, init=<tree 0x0>, origtype=<tree 0x0>, asmspec_tree=<tree 0x0>) >>> at /export/gnu/import/git/sources/gcc/gcc/c/c-decl.c:4863 >>> #3 0x000000000078d1ed in c_parser_declaration_or_fndef ( >>> parser=0x7ffff15050a8, fndef_ok=false, static_assert_ok=true, >>> empty_ok=true, nested=false, start_attr_ok=true, >>> objc_foreach_object_declaration=0x0, omp_declare_simd_clauses=...) >>> at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:1855 >>> #4 0x000000000078c234 in c_parser_external_declaration (parser=0x7ffff15050a8) >>> at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:1435 >>> #5 0x000000000078be45 in c_parser_translation_unit (parser=0x7ffff15050a8) >>> at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:1322 >>> #6 0x00000000007b3271 in c_parse_file () >>> at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:15440 >>> #7 0x000000000081cb97 in c_common_parse_file () >>> at /export/gnu/import/git/sources/gcc/gcc/c-family/c-opts.c:1059 >>> #8 0x0000000000f27662 in compile_file () >>> at /export/gnu/import/git/sources/gcc/gcc/toplev.c:543 >>> ---Type <return> to continue, or q <return> to quit--- >>> #9 0x0000000000f29baa in do_compile () >>> at /export/gnu/import/git/sources/gcc/gcc/toplev.c:2041 >>> #10 0x0000000000f29df9 in toplev::main (this=0x7fffffffdc90, argc=17, >>> argv=0x7fffffffdd98) >>> at /export/gnu/import/git/sources/gcc/gcc/toplev.c:2142 >>> #11 0x00000000017d8228 in main (argc=17, argv=0x7fffffffdd98) >>> at /export/gnu/import/git/sources/gcc/gcc/main.c:39 >>> >>> Later, it was turned into error_mark_node: >>> >>> Old value = <array_type 0x7ffff15ee150> >>> New value = <error_mark 0x7ffff14fac90> >>> finish_decl (decl=<var_decl 0x7ffff1506990 a>, init_loc=0, init=<tree 0x0>, >>> origtype=<tree 0x0>, asmspec_tree=<tree 0x0>) >>> at /export/gnu/import/git/sources/gcc/gcc/c/c-decl.c:4802 >>> 4802 if (TREE_USED (type)) >>> (gdb) bt >>> #0 finish_decl (decl=<var_decl 0x7ffff1506990 a>, init_loc=0, >>> init=<tree 0x0>, origtype=<tree 0x0>, asmspec_tree=<tree 0x0>) >>> at /export/gnu/import/git/sources/gcc/gcc/c/c-decl.c:4802 >>> #1 0x000000000078d1ed in c_parser_declaration_or_fndef ( >>> parser=0x7ffff15050a8, fndef_ok=false, static_assert_ok=true, >>> empty_ok=true, nested=true, start_attr_ok=true, >>> objc_foreach_object_declaration=0x0, omp_declare_simd_clauses=...) >>> at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:1855 >>> #2 0x0000000000792a23 in c_parser_compound_statement_nostart ( >>> parser=0x7ffff15050a8) >>> at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:4621 >>> #3 0x0000000000792688 in c_parser_compound_statement (parser=0x7ffff15050a8) >>> at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:4532 >>> #4 0x000000000078d5a3 in c_parser_declaration_or_fndef ( >>> parser=0x7ffff15050a8, fndef_ok=true, static_assert_ok=true, >>> empty_ok=true, nested=false, start_attr_ok=true, >>> objc_foreach_object_declaration=0x0, omp_declare_simd_clauses=...) >>> at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:1965 >>> #5 0x000000000078c234 in c_parser_external_declaration (parser=0x7ffff15050a8) >>> at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:1435 >>> #6 0x000000000078be45 in c_parser_translation_unit (parser=0x7ffff15050a8) >>> at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:1322 >>> #7 0x00000000007b3271 in c_parse_file () >>> ---Type <return> to continue, or q <return> to quit--- >>> at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:15440 >>> #8 0x000000000081cb97 in c_common_parse_file () >>> at /export/gnu/import/git/sources/gcc/gcc/c-family/c-opts.c:1059 >>> #9 0x0000000000f27662 in compile_file () >>> at /export/gnu/import/git/sources/gcc/gcc/toplev.c:543 >>> #10 0x0000000000f29baa in do_compile () >>> at /export/gnu/import/git/sources/gcc/gcc/toplev.c:2041 >>> #11 0x0000000000f29df9 in toplev::main (this=0x7fffffffdc90, argc=17, >>> argv=0x7fffffffdd98) >>> at /export/gnu/import/git/sources/gcc/gcc/toplev.c:2142 >>> #12 0x00000000017d8228 in main (argc=17, argv=0x7fffffffdd98) >>> at /export/gnu/import/git/sources/gcc/gcc/main.c:39 >>> (gdb) >>> >>> I guess it isn't worth to take it out of varpool. >>> >>> Is my patch OK for trunk? >> >> I don't see why the C FE would need to invoke finish_decl twice here. >> So I'd rather >> have the C frontend not invoke rest_of_decl_compilation on this in the >> first place. >> >> Your patch still feels like a hack in the wrong place. >> > > I changed bug report to C and leave it to C FE people. Thanks. Richard. > > -- > H.J. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Limit alignment on error_mark_node variable 2015-07-09 13:57 ` Richard Biener 2015-07-09 14:06 ` H.J. Lu @ 2015-07-10 12:19 ` H.J. Lu 2015-07-10 12:38 ` Richard Biener 1 sibling, 1 reply; 10+ messages in thread From: H.J. Lu @ 2015-07-10 12:19 UTC (permalink / raw) To: Richard Biener; +Cc: GCC Patches On Thu, Jul 09, 2015 at 03:57:31PM +0200, Richard Biener wrote: > On Thu, Jul 9, 2015 at 1:08 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > > On Thu, Jul 9, 2015 at 2:54 AM, Richard Biener > > <richard.guenther@gmail.com> wrote: > >> On Thu, Jul 9, 2015 at 11:52 AM, H.J. Lu <hjl.tools@gmail.com> wrote: > >>> On Thu, Jul 09, 2015 at 10:16:38AM +0200, Richard Biener wrote: > >>>> On Wed, Jul 8, 2015 at 5:32 PM, H.J. Lu <hongjiu.lu@intel.com> wrote: > >>>> > There is no need to try different alignment on variable of > >>>> > error_mark_node. > >>>> > > >>>> > OK for trunk if there is no regression? > >>>> > >>>> Can't we avoid calling align_variable on error_mark_node type decls > >>>> completely? That is, punt earlier when we try to emit it. > >>>> > >>> > >>> How about this? OK for trunk? > >> > >> Heh, you now get the obvious question why we can't simply avoid > >> adding the varpool node in the first place ;) > >> > > > > When it was first added to varpool, its type was OK: > > > > (gdb) bt > > #0 varpool_node::get_create (decl=<var_decl 0x7ffff1506900 vv>) > > at /export/gnu/import/git/sources/gcc/gcc/varpool.c:150 > > #1 0x0000000000e1c3e8 in rest_of_decl_compilation ( > > decl=<var_decl 0x7ffff1506900 vv>, top_level=1, at_end=0) > > at /export/gnu/import/git/sources/gcc/gcc/passes.c:271 > > #2 0x0000000000731d39 in finish_decl (decl=<var_decl 0x7ffff1506900 vv>, > > init_loc=0, init=<tree 0x0>, origtype=<tree 0x0>, asmspec_tree=<tree 0x0>) > > at /export/gnu/import/git/sources/gcc/gcc/c/c-decl.c:4863 > > #3 0x000000000078d1ed in c_parser_declaration_or_fndef ( > > parser=0x7ffff15050a8, fndef_ok=false, static_assert_ok=true, > > empty_ok=true, nested=false, start_attr_ok=true, > > objc_foreach_object_declaration=0x0, omp_declare_simd_clauses=...) > > at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:1855 > > #4 0x000000000078c234 in c_parser_external_declaration (parser=0x7ffff15050a8) > > at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:1435 > > #5 0x000000000078be45 in c_parser_translation_unit (parser=0x7ffff15050a8) > > at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:1322 > > #6 0x00000000007b3271 in c_parse_file () > > at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:15440 > > #7 0x000000000081cb97 in c_common_parse_file () > > at /export/gnu/import/git/sources/gcc/gcc/c-family/c-opts.c:1059 > > #8 0x0000000000f27662 in compile_file () > > at /export/gnu/import/git/sources/gcc/gcc/toplev.c:543 > > ---Type <return> to continue, or q <return> to quit--- > > #9 0x0000000000f29baa in do_compile () > > at /export/gnu/import/git/sources/gcc/gcc/toplev.c:2041 > > #10 0x0000000000f29df9 in toplev::main (this=0x7fffffffdc90, argc=17, > > argv=0x7fffffffdd98) > > at /export/gnu/import/git/sources/gcc/gcc/toplev.c:2142 > > #11 0x00000000017d8228 in main (argc=17, argv=0x7fffffffdd98) > > at /export/gnu/import/git/sources/gcc/gcc/main.c:39 > > > > Later, it was turned into error_mark_node: > > > > Old value = <array_type 0x7ffff15ee150> > > New value = <error_mark 0x7ffff14fac90> > > finish_decl (decl=<var_decl 0x7ffff1506990 a>, init_loc=0, init=<tree 0x0>, > > origtype=<tree 0x0>, asmspec_tree=<tree 0x0>) > > at /export/gnu/import/git/sources/gcc/gcc/c/c-decl.c:4802 > > 4802 if (TREE_USED (type)) > > (gdb) bt > > #0 finish_decl (decl=<var_decl 0x7ffff1506990 a>, init_loc=0, > > init=<tree 0x0>, origtype=<tree 0x0>, asmspec_tree=<tree 0x0>) > > at /export/gnu/import/git/sources/gcc/gcc/c/c-decl.c:4802 > > #1 0x000000000078d1ed in c_parser_declaration_or_fndef ( > > parser=0x7ffff15050a8, fndef_ok=false, static_assert_ok=true, > > empty_ok=true, nested=true, start_attr_ok=true, > > objc_foreach_object_declaration=0x0, omp_declare_simd_clauses=...) > > at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:1855 > > #2 0x0000000000792a23 in c_parser_compound_statement_nostart ( > > parser=0x7ffff15050a8) > > at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:4621 > > #3 0x0000000000792688 in c_parser_compound_statement (parser=0x7ffff15050a8) > > at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:4532 > > #4 0x000000000078d5a3 in c_parser_declaration_or_fndef ( > > parser=0x7ffff15050a8, fndef_ok=true, static_assert_ok=true, > > empty_ok=true, nested=false, start_attr_ok=true, > > objc_foreach_object_declaration=0x0, omp_declare_simd_clauses=...) > > at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:1965 > > #5 0x000000000078c234 in c_parser_external_declaration (parser=0x7ffff15050a8) > > at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:1435 > > #6 0x000000000078be45 in c_parser_translation_unit (parser=0x7ffff15050a8) > > at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:1322 > > #7 0x00000000007b3271 in c_parse_file () > > ---Type <return> to continue, or q <return> to quit--- > > at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:15440 > > #8 0x000000000081cb97 in c_common_parse_file () > > at /export/gnu/import/git/sources/gcc/gcc/c-family/c-opts.c:1059 > > #9 0x0000000000f27662 in compile_file () > > at /export/gnu/import/git/sources/gcc/gcc/toplev.c:543 > > #10 0x0000000000f29baa in do_compile () > > at /export/gnu/import/git/sources/gcc/gcc/toplev.c:2041 > > #11 0x0000000000f29df9 in toplev::main (this=0x7fffffffdc90, argc=17, > > argv=0x7fffffffdd98) > > at /export/gnu/import/git/sources/gcc/gcc/toplev.c:2142 > > #12 0x00000000017d8228 in main (argc=17, argv=0x7fffffffdd98) > > at /export/gnu/import/git/sources/gcc/gcc/main.c:39 > > (gdb) > > > > I guess it isn't worth to take it out of varpool. > > > > Is my patch OK for trunk? > Let me give it another try. > I don't see why the C FE would need to invoke finish_decl twice here. finish_decl is called once on int vv; and the second time on static int a[vv]; which leads to error_mark_node decl. > So I'd rather > have the C frontend not invoke rest_of_decl_compilation on this in the > first place. > > Your patch still feels like a hack in the wrong place. > This patch avoids calling varpool_node::finalize_decl on error_mark_node type decls. Does it make sense? H.J. -- gcc/ PR target/66810 * cgraphbuild.c (pass_build_cgraph_edges::execute): Skip error_mark_node type decls. gcc/testsuite/ PR target/66810 * gcc.target/i386/pr66810.c: New test. --- gcc/cgraphbuild.c | 3 ++- gcc/testsuite/gcc.target/i386/pr66810.c | 10 ++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gcc.target/i386/pr66810.c diff --git a/gcc/cgraphbuild.c b/gcc/cgraphbuild.c index 7d2d096..4d679d9 100644 --- a/gcc/cgraphbuild.c +++ b/gcc/cgraphbuild.c @@ -381,7 +381,8 @@ pass_build_cgraph_edges::execute (function *fun) FOR_EACH_LOCAL_DECL (fun, ix, decl) if (TREE_CODE (decl) == VAR_DECL && (TREE_STATIC (decl) && !DECL_EXTERNAL (decl)) - && !DECL_HAS_VALUE_EXPR_P (decl)) + && !DECL_HAS_VALUE_EXPR_P (decl) + && TREE_TYPE (decl) != error_mark_node) varpool_node::finalize_decl (decl); record_eh_tables (node, fun); diff --git a/gcc/testsuite/gcc.target/i386/pr66810.c b/gcc/testsuite/gcc.target/i386/pr66810.c new file mode 100644 index 0000000..4778b37 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr66810.c @@ -0,0 +1,10 @@ +/* { dg-do compile { target ia32 } } */ +/* { dg-options "-mno-sse -mno-mmx -miamcu" } */ + +int vv; + +void +i (void) +{ + static int a[vv]; /* { dg-error "storage size" } */ +} -- 2.4.3 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Limit alignment on error_mark_node variable 2015-07-10 12:19 ` H.J. Lu @ 2015-07-10 12:38 ` Richard Biener 0 siblings, 0 replies; 10+ messages in thread From: Richard Biener @ 2015-07-10 12:38 UTC (permalink / raw) To: H.J. Lu, Jan Hubicka; +Cc: GCC Patches On Fri, Jul 10, 2015 at 2:19 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Thu, Jul 09, 2015 at 03:57:31PM +0200, Richard Biener wrote: >> On Thu, Jul 9, 2015 at 1:08 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >> > On Thu, Jul 9, 2015 at 2:54 AM, Richard Biener >> > <richard.guenther@gmail.com> wrote: >> >> On Thu, Jul 9, 2015 at 11:52 AM, H.J. Lu <hjl.tools@gmail.com> wrote: >> >>> On Thu, Jul 09, 2015 at 10:16:38AM +0200, Richard Biener wrote: >> >>>> On Wed, Jul 8, 2015 at 5:32 PM, H.J. Lu <hongjiu.lu@intel.com> wrote: >> >>>> > There is no need to try different alignment on variable of >> >>>> > error_mark_node. >> >>>> > >> >>>> > OK for trunk if there is no regression? >> >>>> >> >>>> Can't we avoid calling align_variable on error_mark_node type decls >> >>>> completely? That is, punt earlier when we try to emit it. >> >>>> >> >>> >> >>> How about this? OK for trunk? >> >> >> >> Heh, you now get the obvious question why we can't simply avoid >> >> adding the varpool node in the first place ;) >> >> >> > >> > When it was first added to varpool, its type was OK: >> > >> > (gdb) bt >> > #0 varpool_node::get_create (decl=<var_decl 0x7ffff1506900 vv>) >> > at /export/gnu/import/git/sources/gcc/gcc/varpool.c:150 >> > #1 0x0000000000e1c3e8 in rest_of_decl_compilation ( >> > decl=<var_decl 0x7ffff1506900 vv>, top_level=1, at_end=0) >> > at /export/gnu/import/git/sources/gcc/gcc/passes.c:271 >> > #2 0x0000000000731d39 in finish_decl (decl=<var_decl 0x7ffff1506900 vv>, >> > init_loc=0, init=<tree 0x0>, origtype=<tree 0x0>, asmspec_tree=<tree 0x0>) >> > at /export/gnu/import/git/sources/gcc/gcc/c/c-decl.c:4863 >> > #3 0x000000000078d1ed in c_parser_declaration_or_fndef ( >> > parser=0x7ffff15050a8, fndef_ok=false, static_assert_ok=true, >> > empty_ok=true, nested=false, start_attr_ok=true, >> > objc_foreach_object_declaration=0x0, omp_declare_simd_clauses=...) >> > at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:1855 >> > #4 0x000000000078c234 in c_parser_external_declaration (parser=0x7ffff15050a8) >> > at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:1435 >> > #5 0x000000000078be45 in c_parser_translation_unit (parser=0x7ffff15050a8) >> > at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:1322 >> > #6 0x00000000007b3271 in c_parse_file () >> > at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:15440 >> > #7 0x000000000081cb97 in c_common_parse_file () >> > at /export/gnu/import/git/sources/gcc/gcc/c-family/c-opts.c:1059 >> > #8 0x0000000000f27662 in compile_file () >> > at /export/gnu/import/git/sources/gcc/gcc/toplev.c:543 >> > ---Type <return> to continue, or q <return> to quit--- >> > #9 0x0000000000f29baa in do_compile () >> > at /export/gnu/import/git/sources/gcc/gcc/toplev.c:2041 >> > #10 0x0000000000f29df9 in toplev::main (this=0x7fffffffdc90, argc=17, >> > argv=0x7fffffffdd98) >> > at /export/gnu/import/git/sources/gcc/gcc/toplev.c:2142 >> > #11 0x00000000017d8228 in main (argc=17, argv=0x7fffffffdd98) >> > at /export/gnu/import/git/sources/gcc/gcc/main.c:39 >> > >> > Later, it was turned into error_mark_node: >> > >> > Old value = <array_type 0x7ffff15ee150> >> > New value = <error_mark 0x7ffff14fac90> >> > finish_decl (decl=<var_decl 0x7ffff1506990 a>, init_loc=0, init=<tree 0x0>, >> > origtype=<tree 0x0>, asmspec_tree=<tree 0x0>) >> > at /export/gnu/import/git/sources/gcc/gcc/c/c-decl.c:4802 >> > 4802 if (TREE_USED (type)) >> > (gdb) bt >> > #0 finish_decl (decl=<var_decl 0x7ffff1506990 a>, init_loc=0, >> > init=<tree 0x0>, origtype=<tree 0x0>, asmspec_tree=<tree 0x0>) >> > at /export/gnu/import/git/sources/gcc/gcc/c/c-decl.c:4802 >> > #1 0x000000000078d1ed in c_parser_declaration_or_fndef ( >> > parser=0x7ffff15050a8, fndef_ok=false, static_assert_ok=true, >> > empty_ok=true, nested=true, start_attr_ok=true, >> > objc_foreach_object_declaration=0x0, omp_declare_simd_clauses=...) >> > at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:1855 >> > #2 0x0000000000792a23 in c_parser_compound_statement_nostart ( >> > parser=0x7ffff15050a8) >> > at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:4621 >> > #3 0x0000000000792688 in c_parser_compound_statement (parser=0x7ffff15050a8) >> > at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:4532 >> > #4 0x000000000078d5a3 in c_parser_declaration_or_fndef ( >> > parser=0x7ffff15050a8, fndef_ok=true, static_assert_ok=true, >> > empty_ok=true, nested=false, start_attr_ok=true, >> > objc_foreach_object_declaration=0x0, omp_declare_simd_clauses=...) >> > at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:1965 >> > #5 0x000000000078c234 in c_parser_external_declaration (parser=0x7ffff15050a8) >> > at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:1435 >> > #6 0x000000000078be45 in c_parser_translation_unit (parser=0x7ffff15050a8) >> > at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:1322 >> > #7 0x00000000007b3271 in c_parse_file () >> > ---Type <return> to continue, or q <return> to quit--- >> > at /export/gnu/import/git/sources/gcc/gcc/c/c-parser.c:15440 >> > #8 0x000000000081cb97 in c_common_parse_file () >> > at /export/gnu/import/git/sources/gcc/gcc/c-family/c-opts.c:1059 >> > #9 0x0000000000f27662 in compile_file () >> > at /export/gnu/import/git/sources/gcc/gcc/toplev.c:543 >> > #10 0x0000000000f29baa in do_compile () >> > at /export/gnu/import/git/sources/gcc/gcc/toplev.c:2041 >> > #11 0x0000000000f29df9 in toplev::main (this=0x7fffffffdc90, argc=17, >> > argv=0x7fffffffdd98) >> > at /export/gnu/import/git/sources/gcc/gcc/toplev.c:2142 >> > #12 0x00000000017d8228 in main (argc=17, argv=0x7fffffffdd98) >> > at /export/gnu/import/git/sources/gcc/gcc/main.c:39 >> > (gdb) >> > >> > I guess it isn't worth to take it out of varpool. >> > >> > Is my patch OK for trunk? >> > > Let me give it another try. > >> I don't see why the C FE would need to invoke finish_decl twice here. > > finish_decl is called once on > > int vv; > > and the second time on > > static int a[vv]; > > which leads to error_mark_node decl. > >> So I'd rather >> have the C frontend not invoke rest_of_decl_compilation on this in the >> first place. >> >> Your patch still feels like a hack in the wrong place. >> > > This patch avoids calling varpool_node::finalize_decl on error_mark_node > type decls. Does it make sense? I think that makes sense (though I wonder why this loop is here and not, say, in cgraph_node::finalize_function - but that's for somebody else to cleanup). Honza? Richard. > > H.J. > -- > gcc/ > > PR target/66810 > * cgraphbuild.c (pass_build_cgraph_edges::execute): Skip > error_mark_node type decls. > > gcc/testsuite/ > > PR target/66810 > * gcc.target/i386/pr66810.c: New test. > --- > gcc/cgraphbuild.c | 3 ++- > gcc/testsuite/gcc.target/i386/pr66810.c | 10 ++++++++++ > 2 files changed, 12 insertions(+), 1 deletion(-) > create mode 100644 gcc/testsuite/gcc.target/i386/pr66810.c > > diff --git a/gcc/cgraphbuild.c b/gcc/cgraphbuild.c > index 7d2d096..4d679d9 100644 > --- a/gcc/cgraphbuild.c > +++ b/gcc/cgraphbuild.c > @@ -381,7 +381,8 @@ pass_build_cgraph_edges::execute (function *fun) > FOR_EACH_LOCAL_DECL (fun, ix, decl) > if (TREE_CODE (decl) == VAR_DECL > && (TREE_STATIC (decl) && !DECL_EXTERNAL (decl)) > - && !DECL_HAS_VALUE_EXPR_P (decl)) > + && !DECL_HAS_VALUE_EXPR_P (decl) > + && TREE_TYPE (decl) != error_mark_node) > varpool_node::finalize_decl (decl); > record_eh_tables (node, fun); > > diff --git a/gcc/testsuite/gcc.target/i386/pr66810.c b/gcc/testsuite/gcc.target/i386/pr66810.c > new file mode 100644 > index 0000000..4778b37 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/i386/pr66810.c > @@ -0,0 +1,10 @@ > +/* { dg-do compile { target ia32 } } */ > +/* { dg-options "-mno-sse -mno-mmx -miamcu" } */ > + > +int vv; > + > +void > +i (void) > +{ > + static int a[vv]; /* { dg-error "storage size" } */ > +} > -- > 2.4.3 > ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2015-07-10 12:38 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-07-08 15:32 [PATCH] Limit alignment on error_mark_node variable H.J. Lu 2015-07-09 8:16 ` Richard Biener 2015-07-09 9:52 ` H.J. Lu 2015-07-09 9:54 ` Richard Biener 2015-07-09 11:08 ` H.J. Lu 2015-07-09 13:57 ` Richard Biener 2015-07-09 14:06 ` H.J. Lu 2015-07-09 14:10 ` Richard Biener 2015-07-10 12:19 ` H.J. Lu 2015-07-10 12:38 ` Richard Biener
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).