* [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).