From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from rock.gnat.com (rock.gnat.com [IPv6:2620:20:4000:0:a9e:1ff:fe9b:1d1]) by sourceware.org (Postfix) with ESMTPS id 2D58C385700D for ; Tue, 31 May 2022 13:27:17 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 2D58C385700D Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 469AD1169E2; Tue, 31 May 2022 09:27:15 -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 KGw-n1s5evHo; Tue, 31 May 2022 09:27:15 -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 CA28D116999; Tue, 31 May 2022 09:27:14 -0400 (EDT) Received: from livre (livre.home [172.31.160.2]) by free.home (8.15.2/8.15.2) with ESMTPS id 24VDR5vo717537 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 31 May 2022 10:27:06 -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: Tue, 31 May 2022 10:27:05 -0300 In-Reply-To: (Richard Biener's message of "Mon, 30 May 2022 14:21:32 +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: Tue, 31 May 2022 13:27:18 -0000 On May 30, 2022, Richard Biener wrote: > I don't think you can rely on TREE_VISITED not set at the start of the > pass (and you don't clear it either). I don't clear it, but I loop over all SSA names and set TREE_VISITED to either true or false, so that's covered. I even had a test patch that checked that TREE_VISITED remains unchanged and still matched the expected value, with a recursive verification. I could switch to an sbitmap if that's preferred, though. > I also wonder how you decide that tracking PHIs with (one) uninit arg > is "enough"? It's a conservative assumption, granted. One could imagine cases in which an uninit def is never actually used, say because of conditionals forced by external circumstances the compiler cannot possibly know about. But then, just as this sort of bug shows, sometimes even when an uninit is not actually used, the fact that it is uninit and thus undefined may end up percolating onto stuff that is actually used, so I figured we'd be better off leaving alone whatever is potentially derived from an uninit value. > Is it important which edge of the PHI the undef appears in? At some point, I added recursion to find_ssa_undef, at PHI nodes and assignments, and pondered whether to recurse at PHI nodes only for defs that were "earlier" ones, rather than coming from back edges. I ended up reasoning that back edges would affect step and rule out an IV candidate even sooner. But the forward propagation of maybe-undef obviated that reasoning. Now, almost tautologically, if circumstances are such that the compiler could only tell that an ssa name is defined with external knowledge, then, since such external knowledge is not available to the compiler, it has to assume that the ssa name may be undefined. > I presume the testcase might have it on the loop entry edge? The original testcase did. The modified one (the added increment) shows it can be an earlier edge that has the maybe-undef name. > I presume only PHIs in loop headers are to be considered? As in the modified testcase, earlier PHIs that are entirely outside the loop can still trigger the bug. Adding more increments of g guarded by conditionals involving other global variables pushes the undef ssa name further and further away from the inner loop, while still rendering g an unsuitable IV. >> +int a, b, c[1], d[2], *e = c; >> +int main() { >> + int f = 0; >> + for (; b < 2; b++) { >> + int g; >> + if (f) >> + g++, b = 40; >> + a = d[b * b]; >> + for (f = 0; f < 3; f++) { >> + if (e) >> + break; >> + g--; >> + if (a) >> + a = g; >> + } >> + } >> + return 0; >> +} -- 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