From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ciao.gmane.io (ciao.gmane.io [116.202.254.214]) by sourceware.org (Postfix) with ESMTPS id 6F2B43857726 for ; Mon, 6 Nov 2023 18:20:38 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 6F2B43857726 Authentication-Results: sourceware.org; dmarc=fail (p=none dis=none) header.from=gmx.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=m.gmane-mx.org ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 6F2B43857726 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=116.202.254.214 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1699294839; cv=none; b=sjpv/vGxosds3wVVDHT+Vx7cnUFVhjlTQDtCXE1ePpYSuAMk5pxyNAmHvKBYpaOEW7/et6F8PxooXuT+6GepieIzLenQavT23Cs2I76B7cHiMh9YsoFfMRe6pg5NceBkF3gHqqdM7WXJiktAM0noFol/uWzdXAKKCJinr5nU9ZU= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1699294839; c=relaxed/simple; bh=uQjvw8nIcqnmEfpXtPKe6DffAIK+qj1hlrN0IaR0joE=; h=To:From:Subject:Date:Message-ID:Mime-Version; b=aXqh9NDkOYtYVcJsKFc1ttfCWXfglJ2yFYuezlz7S2R29e4pEBiFaoQmmioihC2GsUZmQ4kqqJKYQB1z3nNhmDgXVuYegim/wkvKuo7D9jQ7YFQx2LkX9sjulgRyjMjA2AdoBRhv60BigL6EwYmUxVpIfnYwqxkIviBEMtAG+1g= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from list by ciao.gmane.io with local (Exim 4.92) (envelope-from ) id 1r04DF-00076Z-0L for gcc-patches@gcc.gnu.org; Mon, 06 Nov 2023 19:20:37 +0100 X-Injected-Via-Gmane: http://gmane.org/ To: gcc-patches@gcc.gnu.org From: Harald Anlauf Subject: Re: [PATCH 2/2] libgfortran: Remove empty array descriptor first dimension overwrite [PR112371] Date: Mon, 6 Nov 2023 19:20:32 +0100 Message-ID: <5562f6b7-1b0d-4ff7-9797-8f1f58edc1f5@gmx.de> References: <20231106114325.828968-1-mikael@gcc.gnu.org> <20231106114325.828968-3-mikael@gcc.gnu.org> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit User-Agent: Mozilla Thunderbird Content-Language: en-US In-Reply-To: <20231106114325.828968-3-mikael@gcc.gnu.org> Cc: fortran@gcc.gnu.org X-Spam-Status: No, score=-9.0 required=5.0 tests=BAYES_00,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM,GIT_PATCH_0,HEADER_FROM_DIFFERENT_DOMAINS,KAM_DMARC_STATUS,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Message-ID: <20231106182032.gOvwtcLnkm62-NnDytrZVU0iLoK_Y-QiOe-fhcNNS4s@z> Hi Mikael, Am 06.11.23 um 12:43 schrieb Mikael Morin: > Remove the forced overwrite of the first dimension of the result array > descriptor to set it to zero extent, in the function templates for > transformational functions doing an array reduction along a dimension. This > overwrite, which happened before early returning in case the result array > was empty, was wrong because an array may have a non-zero extent in the > first dimension and still be empty if it has a zero extent in a higher > dimension. Overwriting the dimension was resulting in wrong array result > upper bound for the first dimension in that case. > > The offending piece of code was present in several places, and this removes > them all. More precisely, there is only one case to fix for logical > reduction functions, and there are three cases for other reduction > functions, corresponding to non-masked reduction, reduction with array mask, > and reduction with scalar mask. The impacted m4 files are > ifunction_logical.m4 for logical reduction functions, ifunction.m4 for > regular functions and types, ifunction-s.m4 for character minloc and maxloc, > ifunction-s2.m4 for character minval and maxval, and ifindloc1.m4 for > findloc. while your fix seems mechanical and correct, I wonder if you looked at the following pre-existing irregularity which can be seen in this snippet: > diff --git a/libgfortran/m4/ifunction.m4 b/libgfortran/m4/ifunction.m4 > index 480649cf691..abc15b430ab 100644 > --- a/libgfortran/m4/ifunction.m4 > +++ b/libgfortran/m4/ifunction.m4 > @@ -96,12 +96,7 @@ name`'rtype_qual`_'atype_code` ('rtype` * const restrict retarray, > > retarray->base_addr = xmallocarray (alloc_size, sizeof (rtype_name)); > if (alloc_size == 0) > - { > - /* Make sure we have a zero-sized array. */ > - GFC_DIMENSION_SET(retarray->dim[0], 0, -1, 1); > - return; > - > - } > + return; > } > else > { This is all enclosed in a block which has if (retarray->base_addr == NULL) but allocates and sets retarray->base_addr, while > @@ -290,11 +285,7 @@ m'name`'rtype_qual`_'atype_code` ('rtype` * const restrict retarray, > retarray->dtype.rank = rank; > > if (alloc_size == 0) > - { > - /* Make sure we have a zero-sized array. */ > - GFC_DIMENSION_SET(retarray->dim[0], 0, -1, 1); > - return; > - } > + return; > else > retarray->base_addr = xmallocarray (alloc_size, sizeof (rtype_name)); > and > @@ -454,11 +445,7 @@ void > alloc_size = GFC_DESCRIPTOR_STRIDE(retarray,rank-1) * extent[rank-1]; > > if (alloc_size == 0) > - { > - /* Make sure we have a zero-sized array. */ > - GFC_DIMENSION_SET(retarray->dim[0], 0, -1, 1); > - return; > - } > + return; > else > retarray->base_addr = xmallocarray (alloc_size, sizeof (rtype_name)); > } do not set retarray->base_addr to non-NULL for alloc_size == 0. Do you know if the first snippet can be safely rewritten to avoid the (hopefully pointless) xmallocarray for alloc_size == 0? Thanks, Harald