This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

PING^2: [PATCH] Limit alignment on error_mark_node variable


PING

On Wed, Sep 30, 2015 at 9:13 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> PING
>
> On Fri, Jul 10, 2015 at 5:19 AM, 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?
>>
>>
>> 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
>>
>
>
>
> --
> H.J.



-- 
H.J.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]