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 3070038582B6 for ; Mon, 17 Oct 2022 14:46:40 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 3070038582B6 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=codesourcery.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=mentor.com X-IronPort-AV: E=Sophos;i="5.95,191,1661846400"; d="scan'208,223";a="84886125" Received: from orw-gwy-02-in.mentorg.com ([192.94.38.167]) by esa4.mentor.iphmx.com with ESMTP; 17 Oct 2022 06:46:39 -0800 IronPort-SDR: yHzHY375Nr2k4tp0svekYWiqMFRxPTXvDhVHkgEm7WT6rkWkw3ewu9WyAWhsj3QplBed/LwEJ0 OP9REEOmNRNTuXrZyv/UNN8+kCegE/G7p7HXwq7i0Rhn8S6c6/vpNeAl2keok7t0tPkzSYylGM c+LqmZeXFE4kFH5CpMQS0JtVSiWdWjyz5s+kO+qSBq34QU/zD/r5ob/04O2JrJsycmIOWxqOTs nVjPXk2gDqCxI3EHEEugcuUBIEVooRKNm0Of8pX2sLxZ+INkCSbtgzwuzmVpFBIsLyQ8d13NLY 7hY= From: Thomas Schwinge To: Aldy Hernandez , Subject: Re: Add 'c-c++-common/torture/pr107195-1.c' [PR107195] (was: [COMMITTED] [PR107195] Set range to zero when nonzero mask is 0.) In-Reply-To: References: <20221011083137.336470-1-aldyh@redhat.com> <878rlej3o6.fsf@euler.schwinge.homeip.net> User-Agent: Notmuch/0.29.1+93~g67ed7df (https://notmuchmail.org) Emacs/26.3 (x86_64-pc-linux-gnu) Date: Mon, 17 Oct 2022 16:46:32 +0200 Message-ID: <87o7uafqyf.fsf@dem-tschwing-1.ger.mentorg.com> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Originating-IP: [137.202.0.90] X-ClientProxiedBy: svr-ies-mbx-11.mgc.mentorg.com (139.181.222.11) To svr-ies-mbx-10.mgc.mentorg.com (139.181.222.10) X-Spam-Status: No, score=-11.8 required=5.0 tests=BAYES_00,GIT_PATCH_0,HEADER_FROM_DIFFERENT_DOMAINS,KAM_DMARC_STATUS,KAM_LOTSOFHASH,SPF_HELO_PASS,SPF_PASS,TXREP 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: --=-=-= Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Hi! On 2022-10-17T15:58:47+0200, Aldy Hernandez wrote: > On Mon, Oct 17, 2022 at 9:44 AM Thomas Schwinge = wrote: >> On 2022-10-11T10:31:37+0200, Aldy Hernandez via Gcc-patches wrote: >> > When solving 0 =3D _15 & 1, we calculate _15 as: >> > >> > [irange] int [-INF, -2][0, +INF] NONZERO 0xfffffffe >> > >> > The known value of _15 is [0, 1] NONZERO 0x1 which is intersected with >> > the above, yielding: >> > >> > [0, 1] NONZERO 0x0 >> > >> > This eventually gets copied to a _Bool [0, 1] NONZERO 0x0. >> > >> > This is problematic because here we have a bool which is zero, but >> > returns false for irange::zero_p, since the latter does not look at >> > nonzero bits. This causes logical_combine to assume the range is >> > not-zero, and all hell breaks loose. >> > >> > I think we should just normalize a nonzero mask of 0 to [0, 0] at >> > creation, thus avoiding all this. >> >> 1. This commit r13-3217-gc4d15dddf6b9eacb36f535807ad2ee364af46e04 >> "[PR107195] Set range to zero when nonzero mask is 0" broke a GCC/nvptx >> offloading test case: >> >> UNSUPPORTED: libgomp.oacc-c/../libgomp.oacc-c-c++-common/nvptx-sese-= 1.c -DACC_DEVICE_TYPE_nvidia=3D1 -DACC_MEM_SHARED=3D0 -foffload=3Dnvptx-non= e -O0 >> PASS: libgomp.oacc-c/../libgomp.oacc-c-c++-common/nvptx-sese-1.c -DA= CC_DEVICE_TYPE_nvidia=3D1 -DACC_MEM_SHARED=3D0 -foffload=3Dnvptx-none -O2 = (test for excess errors) >> PASS: libgomp.oacc-c/../libgomp.oacc-c-c++-common/nvptx-sese-1.c -DA= CC_DEVICE_TYPE_nvidia=3D1 -DACC_MEM_SHARED=3D0 -foffload=3Dnvptx-none -O2 = execution test >> [-PASS:-]{+FAIL:+} libgomp.oacc-c/../libgomp.oacc-c-c++-common/nvptx= -sese-1.c -DACC_DEVICE_TYPE_nvidia=3D1 -DACC_MEM_SHARED=3D0 -foffload=3Dnvp= tx-none -O2 scan-nvptx-none-offload-rtl-dump mach "SESE regions:.* [0-9]= +{[0-9]+->[0-9]+(\\.[0-9]+)+}" >> >> Same for C++. >> >> I'll later send a patch (for the test case!) to fix that up. >> >> 2. Looking into this, I found that this >> commit r13-3217-gc4d15dddf6b9eacb36f535807ad2ee364af46e04 >> "[PR107195] Set range to zero when nonzero mask is 0" actually enables a >> code transformation/optimization that GCC apparently has not been doing >> before! I've tried to capture that in the attached >> "Add 'c-c++-common/torture/pr107195-1.c' [PR107195]". > > Nice. > >> Will you please verify that one? In its current '#if 1' configuration, >> it's all-PASS after commit >> r13-3217-gc4d15dddf6b9eacb36f535807ad2ee364af46e04 >> "[PR107195] Set range to zero when nonzero mask is 0", whereas before, w= e >> get two calls to 'foo', because GCC apparently didnn't understand the >> relation (optimization opportunity) between 'r *=3D 2;' and the subseque= nt >> 'if (r & 1)'. > > Yeah, that looks correct. We keep better track of nonzero masks. OK, next observation: this also works for split-up expressions 'if ((r & 2) && (r & 1))' (same rationale as for 'if (r & 1)' alone). I've added such a variant in my test case. But: it doesn't work for logically equal 'if (r & 3)'. I've added such an XFAILed variant in my test case. Do you have guidance what needs to be done to make such cases work, too? >> I've left in the other '#if' variants in case you'd like to experiment >> with these, but would otherwise clean that up before pushing. >> >> Where does one put such a test case? >> >> Should the file be named 'pr107195' or something else? > > The aforementioned patch already has: > > * gcc.dg/tree-ssa/pr107195-1.c: New test. > * gcc.dg/tree-ssa/pr107195-2.c: New test. > > So I would just add a pr107195-3.c test. But note that unlike yours in 'gcc.dg/tree-ssa/', I had put mine into 'c-c++-common/torture/'. That's so that we get C and C++ testing, and all torture testing flag variants. (... where we do see the optimization happen starting at '-O1'.) Do you think that is excessive, and a single 'gcc.dg/tree-ssa/' test case, C only, '-O1' only is sufficient for this? (I don't have much experience with test cases in such regions of GCC, hence these questions.) >> Do we scan 'optimized', or an earlier dump? >> >> At '-O1', the actual code transformation is visible already in the 'dom2= ' >> dump: >> >> [local count: 536870913]: >> gimple_assign >> + gimple_assign >> + goto ; [100.00%] >> >> - [local count: 1073741824]: >> - # gimple_phi >> + [local count: 536870912]: >> + # gimple_phi >> gimple_assign >> gimple_cond >> - goto ; [50.00%] >> + goto ; [100.00%] >> else >> - goto ; [50.00%] >> + goto ; [0.00%] >> >> [local count: 536870913]: >> gimple_call >> gimple_assign >> >> [local count: 1073741824]: >> - # gimple_phi >> + # gimple_phi >> gimple_return >> >> And, the actual "avoid second call 'foo'" optimization is visiable >> starting 'dom3': >> >> [local count: 536870913]: >> gimple_assign >> + goto ; [100.00%] >> >> - [local count: 1073741824]: >> - # gimple_phi >> - gimple_assign >> + [local count: 536870912]: >> + gimple_assign >> gimple_cond >> - goto ; [50.00%] >> + goto ; [100.00%] >> else >> - goto ; [50.00%] >> + goto ; [0.00%] >> >> [local count: 536870913]: >> - gimple_call >> - gimple_assign >> + gimple_assign >> + gimple_assign >> >> [local count: 1073741824]: >> - # gimple_phi >> + # gimple_phi >> gimple_return >> >> ..., but I don't know if either of those would be stable/appropriate to >> scan instead of 'optimized'? > > IMO, either dom3 or optimized is fine. OK, I left it at 'optimized', as I don't have any rationale why exactly it should happen in 'dom3' already. ;-) Gr=C3=BC=C3=9Fe Thomas ----------------- Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstra=C3=9Fe 201= , 80634 M=C3=BCnchen; Gesellschaft mit beschr=C3=A4nkter Haftung; Gesch=C3= =A4ftsf=C3=BChrer: Thomas Heurung, Frank Th=C3=BCrauf; Sitz der Gesellschaf= t: M=C3=BCnchen; Registergericht M=C3=BCnchen, HRB 106955 --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename="0001-Add-c-c-common-torture-pr107195-1.c-PR107195.patch" >From 8b1a33933002ec9e9accdb41229b9b19a431b66a Mon Sep 17 00:00:00 2001 From: Thomas Schwinge Date: Mon, 17 Oct 2022 09:10:03 +0200 Subject: [PATCH] Add 'c-c++-common/torture/pr107195-1.c' [PR107195] ... to display optimization performed as of recent commit r13-3217-gc4d15dddf6b9eacb36f535807ad2ee364af46e04 "[PR107195] Set range to zero when nonzero mask is 0". PR tree-optimization/107195 gcc/testsuite/ * c-c++-common/torture/pr107195-1.c: New. --- .../c-c++-common/torture/pr107195-1.c | 78 +++++++++++++++++++ 1 file changed, 78 insertions(+) create mode 100644 gcc/testsuite/c-c++-common/torture/pr107195-1.c diff --git a/gcc/testsuite/c-c++-common/torture/pr107195-1.c b/gcc/testsuite/c-c++-common/torture/pr107195-1.c new file mode 100644 index 00000000000..477eed6e959 --- /dev/null +++ b/gcc/testsuite/c-c++-common/torture/pr107195-1.c @@ -0,0 +1,78 @@ +/* Inspired by 'libgomp.oacc-c-c++-common/nvptx-sese-1.c'. */ + +/* { dg-additional-options -fdump-tree-optimized-raw } + { dg-skip-if {} { *-*-* } { {-flto -fno-fat-lto-objects} } { } } */ + +extern int +__attribute__((const)) +foo1 (int); + +int f1 (int r) +{ + if (foo1 (r)) /* If this first 'if' holds... */ + r *= 2; /* ..., 'r' now has a zero-value lower-most bit... */ + + if (r & 1) /* ..., so this second 'if' can never hold... */ + { /* ..., so this is unreachable. */ + /* In constrast, if the first 'if' does not hold ('foo1 (r) == 0'), the + second 'if' may hold, but we know ('foo1' being 'const') that + 'foo1 (r) == 0', so don't have to re-evaluate it here: */ + r += foo1 (r); + /* Thus, if optimizing, we only ever expect one call of 'foo1'. + { dg-final { scan-tree-dump-times {gimple_call