From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from rock.gnat.com (rock.gnat.com [205.232.38.15]) by sourceware.org (Postfix) with ESMTPS id 5726C382CF27 for ; Wed, 1 Jun 2022 19:56:20 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 5726C382CF27 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 07101116741; Wed, 1 Jun 2022 15:56:20 -0400 (EDT) X-Virus-Scanned: Debian amavisd-new at gnat.com Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id 9I1IOJLAJp1q; Wed, 1 Jun 2022 15:56:19 -0400 (EDT) Received: from free.home (tron.gnat.com [IPv6:2620:20:4000:0:46a8:42ff:fe0e:e294]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by rock.gnat.com (Postfix) with ESMTPS id 5BC5C116735; Wed, 1 Jun 2022 15:56:19 -0400 (EDT) Received: from livre (free-to-gw.home [172.31.160.161]) by free.home (8.15.2/8.15.2) with ESMTPS id 251Ju9of028322 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 1 Jun 2022 16:56:10 -0300 From: Alexandre Oliva To: Richard Biener Cc: GCC Patches , Bin Cheng Subject: Re: [PATCH] [PR105665] ivopts: check defs of names in base for undefs Organization: Free thinker, does not speak for AdaCore References: Errors-To: aoliva@lxoliva.fsfla.org Date: Wed, 01 Jun 2022 16:56:09 -0300 In-Reply-To: (Richard Biener's message of "Wed, 1 Jun 2022 11:42:25 +0200") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Scanned-By: MIMEDefang 2.84 X-Spam-Status: No, score=-6.3 required=5.0 tests=BAYES_00, 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 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: Wed, 01 Jun 2022 19:56:23 -0000 On Jun 1, 2022, Richard Biener wrote: > On Tue, May 31, 2022 at 3:27 PM Alexandre Oliva wrote: > int i; > if (flag) > i = init; > i++; > still would (the propagation stops at i++). Uh, are you sure? That doesn't sound right. I meant for the propagation to affect the incremented i as well, and any users thereof: the incremented i is maybe-undefined, since its computation involves another maybe-undefined value. > do we understand the failure mode good enough to say that > a must-def (even if from an SSA name without a definition) is good > enough to avoid the issues we are seeing? A must-def computed from a maybe-undef doesn't protect us from the failure. I assumed the failure mode was understood well enough to make directly-visible undefined SSA_NAMEs problematic, and, given the finding that indirectly-visible ones were also problematic, even with multiple-steps removed, I figured other optimizations such as jump threading could turn indirectly-visible undef names into directly-visible ones, or that other passes could take advantage of indirectly-visible undefinedness leading to potential undefined behavior, to the point that we ought to avoid that. > One would argue that my example above invokes undefined behavior > if (!flag), but IIRC the cases in the PRs we talk about are not but > IVOPTs with its IV choice exposes undefined behavior - orignially > by relying on undef - undef being zero. *nod*. Perhaps we could refrain from propagating through assignments, like the i++ increment above, rather than PHIs after the conditional increment in my modified testcase, on the grounds that, if such non-PHI assignments exercised, then we can assume any of its operands are defined, because otherwise undefined behavior would have been invoked. I.e., at least for ivopts purposes, we could propagate maybe-undefined down PHI nodes only. > That said, the contains-undef thing tries to avoid rewriting expressions > with terms that possibly contain undefs which means if we want to > strenthen it then we look for must-defined (currently it's must-undefined)? I went for must-defined in the patch, but by computing its negated form, maybe-undefined. Now I'm thinking we can go for an even stricter predicate to disable the optimization: if a non-PHI use of a maybe-undefined dominates the loop, then we can still perform the optimization: >> >> + int g; >> >> + if (f) >> >> + g++, b = 40; y = g+2; [loop] The mere use of g before the loop requires it to be defined (even though that's impossible above: either path carries the uninitialized value, incremented or not), so we can proceed with the optimization under the assumption that, after undefined behavior, anything goes. This would be more useful for the case you showed, in which there's a conditional initialization followed by an unconditional use. The requirement of no undefined behavior implies the conditional guarding the initializer must hold, so the optimization can be performed. WDYT? -- Alexandre Oliva, happy hacker https://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer Disinformation flourishes because many people care deeply about injustice but very few check the facts. Ask me about