From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from troutmask.apl.washington.edu (troutmask.apl.washington.edu [128.95.76.21]) by sourceware.org (Postfix) with ESMTPS id C3A423858D39; Thu, 22 Feb 2024 21:01:50 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org C3A423858D39 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=troutmask.apl.washington.edu Authentication-Results: sourceware.org; spf=none smtp.mailfrom=troutmask.apl.washington.edu ARC-Filter: OpenARC Filter v1.0.0 sourceware.org C3A423858D39 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=128.95.76.21 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1708635713; cv=none; b=FkXE7tCqV+X5llAmFvdccK6y8eJLxzwV+p3T5NlM0or/Pp4Ig+nlP9BJZKR4NE9mY0CbRe/5DKRhGLjd4selkHvvktu5b9fTppqEWnurdjgZK94U91HmIuyfsC5EdQhDwUG6VgJldFb/yf3lDKTzDXce2BJXPH6luGJJfHBXPiA= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1708635713; c=relaxed/simple; bh=9apwnXBxZNKjhj4m5hDsPRrvJ13fc6ztOY3/IfbUMHg=; h=DKIM-Signature:Date:From:To:Subject:Message-ID:MIME-Version; b=tzvj5MByVIjOoMVYtD8Aoda6bsm22lov+47MD5KxmO1Dlc2w/Bz5jN3AWU/3gPxUVgYJWRvPOhWkglAkVfG076msIoXhFXnumQlSRo7px0Odl7zumUuCZKhGf+zUiVHcKLdvMo2Wgu24/Mjcxd8smgf0yMGj5LgxwYmSzbRR3rA= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from troutmask.apl.washington.edu (localhost [127.0.0.1]) by troutmask.apl.washington.edu (8.17.1/8.17.1) with ESMTP id 41ML1nkt084789; Thu, 22 Feb 2024 13:01:49 -0800 (PST) (envelope-from sgk@troutmask.apl.washington.edu) DKIM-Filter: OpenDKIM Filter v2.10.3 troutmask.apl.washington.edu 41ML1nkt084789 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=troutmask.apl.washington.edu; s=troutmask; t=1708635709; bh=9apwnXBxZNKjhj4m5hDsPRrvJ13fc6ztOY3/IfbUMHg=; h=Date:From:To:Cc:Subject:Reply-To:References:In-Reply-To:From; b=ZrVoi+LN3GtmXOAQbMhAlwBLv6YwLI0S5NxnJ56BqCtizewNHUxXDtnbTmkfaKYBk IsFFhAXJHIhzXn9DV72+Uxiy8pW56Cw/Ib0+YHDPQDBUpGfWb+s8PtaUgDEh2kl44J u74g+3/5SbTwi6oHLj0zBRa7c4sUeAjGZsh9U+MGcR13eWdcWqZ9m96ksxIZ9DfeH0 2kZfwL8lvU7zQDBF5CUWUEscjNpSrtG8I4oKyw1SC7PjQgKfoltDpuvDfsdriCHKaB mc+HJkhndYu5T2VadtN9SMi4StVeqWE7AC03Q/4DdxwN9CgDK9fxME0ToWbhZzBL0F kZueKjypaPf/Q== Received: (from sgk@localhost) by troutmask.apl.washington.edu (8.17.1/8.17.1/Submit) id 41ML1mEU084788; Thu, 22 Feb 2024 13:01:48 -0800 (PST) (envelope-from sgk) Date: Thu, 22 Feb 2024 13:01:48 -0800 From: Steve Kargl To: Harald Anlauf Cc: Jerry D , fortran@gcc.gnu.org, gcc-patches@gcc.gnu.org Subject: Re: [PATCH] Fix fortran/PR114024 Message-ID: Reply-To: sgk@troutmask.apl.washington.edu References: <29ba08a7-8218-4591-8c3f-36c17090e497@gmail.com> <3444d912-2e79-4e16-a425-79810d161ebb@gmx.de> <406c0c3a-f9a4-4f40-a44d-2db284060a59@gmx.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <406c0c3a-f9a4-4f40-a44d-2db284060a59@gmx.de> X-Spam-Status: No, score=-0.7 required=5.0 tests=BAYES_00,DKIM_INVALID,DKIM_SIGNED,KAM_DMARC_STATUS,KAM_NUMSUBJECT,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE,WEIRD_PORT autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Thu, Feb 22, 2024 at 09:22:37PM +0100, Harald Anlauf wrote: > Hi Steve! > > On 2/22/24 01:52, Steve Kargl wrote: > > On Wed, Feb 21, 2024 at 01:42:32PM -0800, Steve Kargl wrote: > > > On Wed, Feb 21, 2024 at 10:20:43PM +0100, Harald Anlauf wrote: > > > > On 2/21/24 22:00, Steve Kargl wrote: > > > > > memleak vs ICE. I think I'll take one over the other. > > > > > Probably need to free code->expr3 before the copy. > > > > > > > > Yep. > > > > > > > > > I tried gfc_replace_expr in an earlier patch. It did not > > > > > work. > > > > I tried freeing code->expr3 before assigning the new expression. > > That leads to > > > > % gfcx -c ~/gcc/gccx/gcc/testsuite/gfortran.dg/allocate_with_source_28.f90 > > pid 69473 comm f951 has trashed its stack, killing > > gfortran: internal compiler error: Illegal instruction signal terminated program f951 > > Right. I also don't see what the lifetimes of the expressions are. > > But is the gfc_copy_expr really needed? Wouldn't the following suffice? > > code->expr3 = gfc_get_parentheses (code->expr3); It's been awhile since I use gfc_copy_expr, gfc_replace_expr, etc. I did not try the above. If that works, then we should use that for simplicity. > > If I don't free code->expr3 but simply assign the new > > expression from gfc_get_parentheses(), your example > > now compiles are executes are expected. It now > > allocate_with_source_28.f90. Caveat: I don't know > > how to test the CLASS uu. > > > > > > > > - it still fails on the following code, because the traversal > > > > > > of the refs is incomplete / wrong: > > > > > > > > > > > > program foo > > > > > > implicit none > > > > > > complex :: cmp(3) > > > > > > real, pointer :: pp(:) > > > > > > class(*), allocatable :: uu(:) > > > > > > type t > > > > > > real :: re > > > > > > real :: im > > > > > > end type t > > > > > > type u > > > > > > type(t) :: tt(3) > > > > > > end type u > > > > > > type(u) :: cc > > > > > > > > > > > > cmp = (3.45,6.78) > > > > > > cc% tt% re = cmp% re > > > > > > cc% tt% im = cmp% im > > > > > > allocate (pp, source = cc% tt% im) ! ICE > > > > > > > > > > cc%tt%im isn't a complex-part-ref, so this seems to > > > > > be a different (maybe related) issue. Does the code > > > > > compile with 'source = (cc%tt%im)'? If so, perhaps, > > > > > detecting a component reference and doing the simply > > > > > wrapping with parentheses can be done. > > > > > > > > Yes, that's why I tried to make up the above example. > > > > I think %re and %im are not too special, they work > > > > here pretty much like component refs elsewhere. > > > > > > > > > > I see. The %re and %im complex-part-ref correspond to > > > ref->u.i == INQUIRY_RE and INQUIRY_IM, respectively. > > > A part-ref for a user-defined type doesn't have an > > > INQUIRY_xxx, so we'll need to see if there is a way to > > > easily identify, e.g., cc%tt%re from your testcase. > > > > The attach patch uses ref->type == REF_COMPONENT to deal > > with the above code. > > I actually wanted to draw your attention away from the > real/complex stuff, because that is not really the point. > When do we actually need to enforce the parentheses? This is essentially my concern. I was inserting parentheses only if I determined they were needed (to avoid unnecessary temporary variable). The code paththat enters the else portion of the following if-else-stmt, where a temporary is created. That is, allocate(x, source=z%re) becomes allocate(x, source=(z%re)) and from code generation viewpoint this is tmp = (z%re) allocate(x, sourcer=tmp) deallocate(tmp) > I tried the following, and it seems to work: > > if (code->expr3->expr_type == EXPR_VARIABLE > && is_subref_array (code->expr3)) > code->expr3 = gfc_get_parentheses (code->expr3); > > (Beware: this is not regtested!) > > On the positive side, it not only seems to fix the cases in question, > but also substring references etc., like the following: If the above passes a regression test, then by all means we should use it. I did not consider the substring case. Even if unneeded parentheses are inserted, which may cause generation of a temporary variable, I hope users are not using 'allocate(x,source=z%re)' is some deeply nested crazy loops structure. BTW, my patch and I suspect your improved patch also fixes 'allocate(x,mold=z%re)'. Consider, complex z(3) real, allocatable :: x(:) z = 42 allocate(x, mold=z%re) print *, size(x) end % gfortran13 -o z a.f90 a.f90:9:25: 9 | allocate(x, mold=z%re) | 1 internal compiler error: in retrieve_last_ref, at fortran/trans-array.cc:6070 0x247d7a679 __libc_start1 /usr/src/lib/libc/csu/libc_start1.c:157 % gfcx -o z a.f90 && ./z 3 -- Steve