From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 114394 invoked by alias); 30 Sep 2015 16:13:21 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 114383 invoked by uid 89); 30 Sep 2015 16:13:20 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.4 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-oi0-f51.google.com Received: from mail-oi0-f51.google.com (HELO mail-oi0-f51.google.com) (209.85.218.51) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Wed, 30 Sep 2015 16:13:18 +0000 Received: by oibi136 with SMTP id i136so25146879oib.3 for ; Wed, 30 Sep 2015 09:13:16 -0700 (PDT) MIME-Version: 1.0 X-Received: by 10.202.2.133 with SMTP id 127mr2731520oic.108.1443629596372; Wed, 30 Sep 2015 09:13:16 -0700 (PDT) Received: by 10.76.175.132 with HTTP; Wed, 30 Sep 2015 09:13:16 -0700 (PDT) Date: Wed, 30 Sep 2015 16:27:00 -0000 Message-ID: Subject: PING: [PATCH] Limit alignment on error_mark_node variable From: "H.J. Lu" To: Richard Biener , Jan Hubicka Cc: GCC Patches Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2015-09/txt/msg02367.txt.bz2 PING On Fri, Jul 10, 2015 at 5:19 AM, H.J. Lu 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 wrote: >> > On Thu, Jul 9, 2015 at 2:54 AM, Richard Biener >> > wrote: >> >> On Thu, Jul 9, 2015 at 11:52 AM, H.J. Lu 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 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=) >> > at /export/gnu/import/git/sources/gcc/gcc/varpool.c:150 >> > #1 0x0000000000e1c3e8 in rest_of_decl_compilation ( >> > decl=, top_level=1, at_end=0) >> > at /export/gnu/import/git/sources/gcc/gcc/passes.c:271 >> > #2 0x0000000000731d39 in finish_decl (decl=, >> > init_loc=0, init=, origtype=, asmspec_tree=) >> > 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 to continue, or q 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 = >> > New value = >> > finish_decl (decl=, init_loc=0, init=, >> > origtype=, asmspec_tree=) >> > at /export/gnu/import/git/sources/gcc/gcc/c/c-decl.c:4802 >> > 4802 if (TREE_USED (type)) >> > (gdb) bt >> > #0 finish_decl (decl=, init_loc=0, >> > init=, origtype=, asmspec_tree=) >> > 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 to continue, or q 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.