From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 23893 invoked by alias); 12 Jan 2013 08:08:22 -0000 Received: (qmail 23861 invoked by uid 22791); 12 Jan 2013 08:08:18 -0000 X-SWARE-Spam-Status: No, hits=-5.9 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,KHOP_RCVD_TRUST,KHOP_THREADED,RCVD_IN_DNSWL_LOW,RCVD_IN_HOSTKARMA_YE X-Spam-Check-By: sourceware.org Received: from mail-ea0-f172.google.com (HELO mail-ea0-f172.google.com) (209.85.215.172) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sat, 12 Jan 2013 08:08:11 +0000 Received: by mail-ea0-f172.google.com with SMTP id f13so1030754eaa.3 for ; Sat, 12 Jan 2013 00:08:10 -0800 (PST) X-Received: by 10.14.204.70 with SMTP id g46mr183178632eeo.15.1357978090096; Sat, 12 Jan 2013 00:08:10 -0800 (PST) Received: from noname (nrbg-4dbe4f8f.pool.mediaWays.net. [77.190.79.143]) by mx.google.com with ESMTPS id 44sm11798932eek.0.2013.01.12.00.08.08 (version=TLSv1 cipher=RC4-SHA bits=128/128); Sat, 12 Jan 2013 00:08:09 -0800 (PST) User-Agent: K-9 Mail for Android In-Reply-To: <20130111194646.GH7269@tucnak.redhat.com> References: <20130111194646.GH7269@tucnak.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Subject: Re: [PATCH] Avoid invalid sharing of ADDR_EXPRs (PR fortran/55935) From: Richard Biener Date: Sat, 12 Jan 2013 08:08:00 -0000 To: Jakub Jelinek ,Jakub Jelinek ,Richard Biener ,fortran@gcc.gnu.org CC: gcc-patches@gcc.gnu.org Message-ID: <3c1c5da9-a6e3-42ea-85fe-cd5385eded37@email.android.com> X-IsSubscribed: yes 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 X-SW-Source: 2013-01/txt/msg00631.txt.bz2 Jakub Jelinek wrote: >Hi! > >As discussed in the PR, the extra verification of location blocks >Richard >posted recently fails on some Fortran testcases. The problem is that >ADDR_EXPRs in static const var initializers contain locations (fixed by >the >trans-expr.c hunks), and that gimple-fold makes the ADDR_EXPRs shared, >so >even with the fortran FE hunks alone when the gimplifier sets location >we get invalid blocks anyway. In the PR we were discussing putting the >unshare_expr at the beginning of canonicalize_constructor_val, >unfortunately >that breaks Ada bootstrap, because it is undesirable when >record_reference >calls that function. So this alternative patch moves the unshare_expr >calls >to gimple-fold.c canonicalize_constructor_val callers. > >Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? Ok. Thanks, Richard. >Alternatively the get_symbol_constant_value unshare_expr call could >move >from canonicalize_constructor_val argument to return val; line, both >locations have some advantages and disadvantages (the one in patch >might >create unnecessary garbage if canonicalize_constructor_val returns NULL >or non-min_invariant, the other way would create garbage with the trees >created by canonicalize_constructor_val itself (they would be created >and immediately unshared). > >2013-01-11 Jakub Jelinek > > PR fortran/55935 > * gimple-fold.c (get_symbol_constant_value): Call > unshare_expr. > (fold_gimple_assign): Don't call unshare_expr here. > (fold_ctor_reference): Call unshare_expr. > > * trans-expr.c (gfc_conv_structure): Call > unshare_expr_without_location on the ctor elements. > >--- gcc/gimple-fold.c.jj 2013-01-11 09:02:55.000000000 +0100 >+++ gcc/gimple-fold.c 2013-01-11 15:42:52.485630537 +0100 >@@ -202,7 +202,7 @@ get_symbol_constant_value (tree sym) > tree val = DECL_INITIAL (sym); > if (val) > { >- val = canonicalize_constructor_val (val, sym); >+ val = canonicalize_constructor_val (unshare_expr (val), sym); > if (val && is_gimple_min_invariant (val)) > return val; > else >@@ -378,7 +378,7 @@ fold_gimple_assign (gimple_stmt_iterator > } > > else if (DECL_P (rhs)) >- return unshare_expr (get_symbol_constant_value (rhs)); >+ return get_symbol_constant_value (rhs); > > /* If we couldn't fold the RHS, hand over to the generic > fold routines. */ >@@ -2941,7 +2941,7 @@ fold_ctor_reference (tree type, tree cto > /* We found the field with exact match. */ > if (useless_type_conversion_p (type, TREE_TYPE (ctor)) > && !offset) >- return canonicalize_constructor_val (ctor, from_decl); >+ return canonicalize_constructor_val (unshare_expr (ctor), >from_decl); > > /* We are at the end of walk, see if we can view convert the > result. */ >@@ -2950,7 +2950,7 @@ fold_ctor_reference (tree type, tree cto > && operand_equal_p (TYPE_SIZE (type), > TYPE_SIZE (TREE_TYPE (ctor)), 0)) > { >- ret = canonicalize_constructor_val (ctor, from_decl); >+ ret = canonicalize_constructor_val (unshare_expr (ctor), >from_decl); > ret = fold_unary (VIEW_CONVERT_EXPR, type, ret); > if (ret) > STRIP_NOPS (ret); >--- gcc/fortran/trans-expr.c.jj 2013-01-11 09:02:50.000000000 +0100 >+++ gcc/fortran/trans-expr.c 2013-01-11 10:43:54.071921147 +0100 >@@ -6137,6 +6137,7 @@ gfc_conv_structure (gfc_se * se, gfc_exp > gfc_symbol *vtabs; > vtabs = cm->initializer->symtree->n.sym; > vtab = gfc_build_addr_expr (NULL_TREE, gfc_get_symbol_decl (vtabs)); >+ vtab = unshare_expr_without_location (vtab); > CONSTRUCTOR_APPEND_ELT (v, cm->backend_decl, vtab); > } > else if (cm->ts.u.derived && strcmp (cm->name, "_size") == 0) >@@ -6150,6 +6151,7 @@ gfc_conv_structure (gfc_se * se, gfc_exp > TREE_TYPE (cm->backend_decl), > cm->attr.dimension, cm->attr.pointer, > cm->attr.proc_pointer); >+ val = unshare_expr_without_location (val); > > /* Append it to the constructor list. */ > CONSTRUCTOR_APPEND_ELT (v, cm->backend_decl, val); > > Jakub