From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from esa4.mentor.iphmx.com (esa4.mentor.iphmx.com [68.232.137.252]) by sourceware.org (Postfix) with ESMTPS id 5A0AD388F063 for ; Mon, 25 May 2020 10:56:25 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 5A0AD388F063 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=codesourcery.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=Thomas_Schwinge@mentor.com IronPort-SDR: JWJpwJ3sZYZT7z79KVlOX/kFma+MJC5NJkdt+eVFjA0dYxzk0AtBtbcHgNXAHOLgmPXYPo29rm mwJpgF4GmW87LieB1QMoOFphvHCbpfbA/b3sDBRTomSr6bAsxYs6JQmfAz9Lo77nhXoLpE4sue 3EkVGCB2cV6CH0QX2jgR4RkweE9HL5sIpujBM6IrDeWE6Zg66wths627hHUTcovFIXuiIors05 eHkETt7PCvrZgIbO4xZxwqSB0sHmS6IJUz05stxM8Idfy+hkztmbT14xYApx4j82eude6c0/EV KNs= X-IronPort-AV: E=Sophos;i="5.73,433,1583222400"; d="scan'208,223";a="49225889" Received: from orw-gwy-02-in.mentorg.com ([192.94.38.167]) by esa4.mentor.iphmx.com with ESMTP; 25 May 2020 02:56:24 -0800 IronPort-SDR: k6Ax1NqM1iHfw24a1Hm+sOBs096wBhUc9wO/x0pSWuAObkp0wqKEJX1r7ZI0EoMZX4KlNwt/pP ZUtSZBVrEFczlCljnqLYfBusbSiHnVM7YprIblr4jVjh9TdVFmoMzzLPimHyAD/gcYJk+lBQQX aeckrR3xuttoGtHLuBfNx8OUPlnMUamobalhdylCmjC3OhpKo0rydJzjEIJ790F65QkUfFRu3H c987eNt8CRm7n/gDaKQ23YHUoiSWWazdgKjiBhcVISsBmGpCSWOn4+6nEXUH9JTBoFlp8UcZQf 3No= From: Thomas Schwinge To: Jakub Jelinek , CC: Sandra Loosemore Subject: [WIP] Fold 'NON_LVALUE_EXPR' some more (was: C++ 'NON_LVALUE_EXPR' in OMP array section handling) In-Reply-To: References: <5a782157-d5ab-e9af-9a6a-78d97fc4fb07@codesourcery.com> User-Agent: Notmuch/0.29.1+93~g67ed7df (https://notmuchmail.org) Emacs/25.2.2 (x86_64-pc-linux-gnu) Date: Mon, 25 May 2020 12:56:15 +0200 Message-ID: MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Originating-IP: [137.202.0.90] X-ClientProxiedBy: SVR-IES-MBX-04.mgc.mentorg.com (139.181.222.4) To SVR-IES-MBX-03.mgc.mentorg.com (139.181.222.3) X-Spam-Status: No, score=-16.3 required=5.0 tests=BAYES_00, GIT_PATCH_0, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 25 May 2020 10:56:27 -0000 --=-=-= Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Hi! Anyone have any input here, especially whether something like the WIP patch attached to generally "Fold 'NON_LVALUE_EXPR' some more" is preferable over local 'STRIP_NOPS'? On 2020-03-26T20:53:19+0100, I wrote: > On 2020-03-26T09:09:01-0600, Sandra Loosemore w= rote: >> On 3/26/20 8:27 AM, Thomas Schwinge wrote: >>> Note that as this code is shared between OpenACC/OpenMP, this might >>> affect OpenMP, too, as far as I can tell. (Subject updated.) Jakub, c= an >>> you please have a look, too? >>> >>> On 2020-03-25T23:02:38-0600, Sandra Loosemore = wrote: >>>> The attached patch fixes a bug I found in the C++ front end's handling >>>> of OpenACC data clauses. The problem here is that the C++ front end >>>> wraps the array bounds expressions in NON_LVALUE_EXPR tree nodes, and >>>> the code here (which appears to have been copied from similar code in >>>> the C front end) was failing to strip those before checking to see if >>>> they were INTEGER_CST nodes, etc. >>> >>> So, I had a quick look. I'm confirming the 'NON_LVALUE_EXPR' (C++ only= , >>> not seen for C) difference, and that 'STRIP_NOPS' gets rid of these. >>> However, I also in some code paths see, for example, 'integer_nonzerop' >>> calls, which internally do 'STRIP_ANY_LOCATION_WRAPPER'. >>> >>> I don't know if 'STRIP_NOPS' is the correct "hammer" to use here, I don= 't >>> know what the usual convention is: explicitly remove (via 'STRIP_NOPS' = as >>> you suggested, or something else), or have the enquiry functions do it >>> ('STRIP_ANY_LOCATION_WRAPPER' as 'integer_nonzerop' is doing, for >>> example). >>> >>> Empirical data doesn't mean too much here, of course, I'm not seeing a >>> lot of explicit 'STRIP_*' calls in 'gcc/cp/semantics.c'. ;-) >> >> I saw that STRIP_NOPS seem to be the preferred way to do things in e.g. >> fold-const.c. I don't know if there's a reason to use some less general >> form of STRIP_* here? >>>> Sadly, I have no test case for this because it was only triggering an >>>> error in conjunction with some other OpenACC patches that are not yet = on >>>> trunk >> In the current code [we have] >> checks like >> >> TREE_CODE (low_bound) =3D=3D INTEGER_CST >> >> etc. So when they're wrapped in NON_LVALUE_EXPRs, it's basically >> failing to detect constants in array dimension specifiers and falling >> through to other code. > > Eh, indeed... ;-\ (So we should be able to deduce some misbehavior from > that, to construct a test case. I'll have a look.) (Have not yet been able to look into constructing any test cases.) > So. I'm not objecting to handling 'NON_LVALUE_EXPR's locally here via > any kind of 'STRIP_*', but it somehow doesn't seem the general solution. > Another option seems to be to teach 'fold_simple' to handle > 'NON_LVALUE_EXPR's, so that the existing code: > > /* We need to reduce to real constant-values for checks below. */ > if (length) > length =3D fold_simple (length); > if (low_bound) > low_bound =3D fold_simple (low_bound); > if (low_bound > && TREE_CODE (low_bound) =3D=3D INTEGER_CST > && [...]) > low_bound =3D fold_convert (sizetype, low_bound); > if (length > && TREE_CODE (length) =3D=3D INTEGER_CST > && [...]) > length =3D fold_convert (sizetype, length); > > ... would then just work. But: I don't know if 'fold_simple' (and > others?) should handle 'NON_LVALUE_EXPR's, or if there's a reason why it > doesn't. (Have not yet tried to figure that out.) What I can tell is > that the attached patch does solve the issue in the C++ OMP array section > handling, and survives a powerpc64le-unknown-linux-gnu > '--enable-checking=3Dyes' (will now re-run with 'fold' checking) bootstra= p, > with no testsuite regressions. > > Hmm. Gr=C3=BC=C3=9Fe Thomas ----------------- Mentor Graphics (Deutschland) GmbH, Arnulfstra=C3=9Fe 201, 80634 M=C3=BCnch= en / Germany Registergericht M=C3=BCnchen HRB 106955, Gesch=C3=A4ftsf=C3=BChrer: Thomas = Heurung, Alexander Walter --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename="0001-Fold-NON_LVALUE_EXPR-some-more.patch" >From 611fbe24b7e459829c0a304a58963d4987c8de0a Mon Sep 17 00:00:00 2001 From: Thomas Schwinge Date: Thu, 26 Mar 2020 21:22:54 +0100 Subject: [PATCH] Fold 'NON_LVALUE_EXPR' some more --- gcc/cp/constexpr.c | 1 + gcc/fold-const.c | 1 + 2 files changed, 2 insertions(+) diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c index 192face9a3a..f31d61c1460 100644 --- a/gcc/cp/constexpr.c +++ b/gcc/cp/constexpr.c @@ -6650,6 +6650,7 @@ fold_simple_1 (tree t) case BIT_NOT_EXPR: case TRUTH_NOT_EXPR: case NOP_EXPR: + case NON_LVALUE_EXPR: case VIEW_CONVERT_EXPR: case CONVERT_EXPR: case FLOAT_EXPR: diff --git a/gcc/fold-const.c b/gcc/fold-const.c index 71a1d3eb735..b6bc5080ff3 100644 --- a/gcc/fold-const.c +++ b/gcc/fold-const.c @@ -1739,6 +1739,7 @@ const_unop (enum tree_code code, tree type, tree arg0) switch (code) { CASE_CONVERT: + case NON_LVALUE_EXPR: case FLOAT_EXPR: case FIX_TRUNC_EXPR: case FIXED_CONVERT_EXPR: -- 2.17.1 --=-=-=--