From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 55C2A3858D32 for ; Tue, 18 Oct 2022 05:41:43 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 55C2A3858D32 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1666071703; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=IOHgVaUsTUFyJXOBLwZeUccUpafT9UkQ6sO7QQKKHoQ=; b=cGt+15LNGoHMLjDF+ae0tv/oUDl7PlR51AJPNj04xYUcgrCMkk25/PzTzreCGOQJ5DkKKD lpoft7KiVq2trIS9xWObYJZ4fMgGc06z8CuU3a6sz/rsAAIasQoday9xvTo+IOF8RexXLi +5LaEt3k6wj6Oyu6TnPecGw9Egm96T0= Received: from mail-yb1-f198.google.com (mail-yb1-f198.google.com [209.85.219.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-606-7BXgenUDN7SpvfdekWgN3Q-1; Tue, 18 Oct 2022 01:41:41 -0400 X-MC-Unique: 7BXgenUDN7SpvfdekWgN3Q-1 Received: by mail-yb1-f198.google.com with SMTP id a2-20020a5b0002000000b006b48689da76so12475794ybp.16 for ; Mon, 17 Oct 2022 22:41:41 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=IOHgVaUsTUFyJXOBLwZeUccUpafT9UkQ6sO7QQKKHoQ=; b=RVHgKw7WmMS/6X0VarfxKDDYTe9i6hHjAmfDDCsZun4uPT9lkLOQpZrlcTwetYPGH8 eOh6rwt/7ooQex4RGkx+R7Ny6I7phy/oDTByvvkpz5gvCWe2na+dRtq7jYuc0keh4mkh aUtaDAk06fMPSoiIB6ZKuk6h+cZBg1swExWFTmEBs3IcKUKSbClJIyp8w4G1ZrLt8UGs J6waNEJL1Qa5KYzB5E109dNvTJDIF+Ze6NPoCoSq66+lxXLCnlAQNQ3qn9ki2UHBXvL5 8LFubTOIaNM8qXByTh91h9vYSjHZOb6QeYE5mCvd2KBiDqamEaGECwZ7IZmbnfJkSh4y NQSA== X-Gm-Message-State: ACrzQf3Q58fF0RiXhHXXNtSjqjP4cw5qkjrLddguAQx6AEjotPxK29TA VsbVwTXWk/5inOVtttl1yD2BteIybPgWnc6PBM3c2WOFxNHgsaGvcBIRBox/MyinsrlHtuAWh6Z Sx4v4GamIWHvoTq0b6FyNpPI69NMgKta8Tg== X-Received: by 2002:a25:23c5:0:b0:6bd:ebf2:ff9d with SMTP id j188-20020a2523c5000000b006bdebf2ff9dmr1092560ybj.80.1666071700990; Mon, 17 Oct 2022 22:41:40 -0700 (PDT) X-Google-Smtp-Source: AMsMyM7+EW+JONrJWYQvTvzGZZ73y2sDK7o0FcEYgMcTZOf7OrUBJCJ0WvNvcwgAFBz4hkg2t9dOhPFp8LobyD/TLtk= X-Received: by 2002:a25:23c5:0:b0:6bd:ebf2:ff9d with SMTP id j188-20020a2523c5000000b006bdebf2ff9dmr1092543ybj.80.1666071700487; Mon, 17 Oct 2022 22:41:40 -0700 (PDT) MIME-Version: 1.0 References: <20221011083137.336470-1-aldyh@redhat.com> <878rlej3o6.fsf@euler.schwinge.homeip.net> <87o7uafqyf.fsf@dem-tschwing-1.ger.mentorg.com> In-Reply-To: <87o7uafqyf.fsf@dem-tschwing-1.ger.mentorg.com> From: Aldy Hernandez Date: Tue, 18 Oct 2022 07:41:29 +0200 Message-ID: Subject: Re: Add 'c-c++-common/torture/pr107195-1.c' [PR107195] (was: [COMMITTED] [PR107195] Set range to zero when nonzero mask is 0.) To: Thomas Schwinge Cc: gcc-patches@gcc.gnu.org X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-5.7 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,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: On Mon, Oct 17, 2022 at 4:47 PM Thomas Schwinge w= rote: > > 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 wi= th > >> > 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/nvpt= x > >> offloading test case: > >> > >> UNSUPPORTED: libgomp.oacc-c/../libgomp.oacc-c-c++-common/nvptx-ses= e-1.c -DACC_DEVICE_TYPE_nvidia=3D1 -DACC_MEM_SHARED=3D0 -foffload=3Dnvptx-n= one -O0 > >> PASS: libgomp.oacc-c/../libgomp.oacc-c-c++-common/nvptx-sese-1.c -= DACC_DEVICE_TYPE_nvidia=3D1 -DACC_MEM_SHARED=3D0 -foffload=3Dnvptx-none -O= 2 (test for excess errors) > >> PASS: libgomp.oacc-c/../libgomp.oacc-c-c++-common/nvptx-sese-1.c -= DACC_DEVICE_TYPE_nvidia=3D1 -DACC_MEM_SHARED=3D0 -foffload=3Dnvptx-none -O= 2 execution test > >> [-PASS:-]{+FAIL:+} libgomp.oacc-c/../libgomp.oacc-c-c++-common/nvp= tx-sese-1.c -DACC_DEVICE_TYPE_nvidia=3D1 -DACC_MEM_SHARED=3D0 -foffload=3Dn= vptx-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 doin= g > >> 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,= we > >> get two calls to 'foo', because GCC apparently didnn't understand the > >> relation (optimization opportunity) between 'r *=3D 2;' and the subseq= uent > >> '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. Unless I'm missing something, your testcase doesn't have a body for foo[123], so GCC has no way to know what any of those functions did or what bits are set/unset. > > 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.) My personal preference is tree-ssa since they are middle end tests. Also, since we're testing ranger, it primarily runs in DOM, VRP, evrp, and the backward threader, so no need to run it at multiple optimization levels. I suggested DOM, because I know ranger runs within DOM, so if the transformation is seen at -O1, it's likely to be done there. Also, evrp/VRP don't run at -O1, so that's another hint it happened in DOM. This is a guess though, it could've been CCP setting a nonzero mask, which then ranger/DOM picked up. All in all, I'm in favor of putting tests as early as possible, otherwise any number of passes could perform a transformation that could lead to the same end result. We are testing ranger, so the most likely place to put this test is in DOM at -O1, or in evrp/VRP[12] for -O2. Of course, this is my personal preference, and these are just general guidelines. Perhaps others can opine. Aldy > > >> Do we scan 'optimized', or an earlier dump? > >> > >> At '-O1', the actual code transformation is visible already in the 'do= m2' > >> 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 t= o > >> 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 2= 01, 80634 M=C3=BCnchen; Gesellschaft mit beschr=C3=A4nkter Haftung; Gesch= =C3=A4ftsf=C3=BChrer: Thomas Heurung, Frank Th=C3=BCrauf; Sitz der Gesellsc= haft: M=C3=BCnchen; Registergericht M=C3=BCnchen, HRB 106955