From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out1.suse.de (smtp-out1.suse.de [IPv6:2a07:de40:b251:101:10:150:64:1]) by sourceware.org (Postfix) with ESMTPS id 4785A3858C53 for ; Tue, 6 Feb 2024 09:00:00 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 4785A3858C53 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.de ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 4785A3858C53 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2a07:de40:b251:101:10:150:64:1 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1707210008; cv=none; b=mImQq7iticsIku9QldzzNiXCg6GWlU61VqYLauyKFXyxTY1aeANwPhnvlbuAZ8A+XZWWJBEMxHmeHq9aXhqdhql4EcxkQEV3UJVXI97nePdqkKk6tACeRI/kLvRNLooi+zyWMnBXY4z/wj6oR6UZpzOhKH7qfsT1yMgw79y+GYI= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1707210008; c=relaxed/simple; bh=emePQz+KXaxmB/C2uz3hMQNpzCI9mXrEVfYjOHOfCNc=; h=DKIM-Signature:DKIM-Signature:DKIM-Signature:DKIM-Signature:Date: From:To:Subject:Message-ID:MIME-Version; b=sVP4Wb41cMnBv7oz10Hpppw4xzCIeC8i0cvOT5W2r8zNBbNZfQ02suCtbZJ4ppQm1XuYoKEz7nHXXRXsLpjrvVqIGhN0C4HBC4wZJ5CwWGEkXF2mFAc1OY3sISx7iIYWkQCnvlUQ8uqtABA6Jgs3kSKRfJCRm9rlasqgKdsufbQ= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from [10.168.4.150] (unknown [10.168.4.150]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id 022D1221BC; Tue, 6 Feb 2024 08:59:59 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1707209999; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=KyrMNiTZMVYb6KNeedcQB3XxOGdJo71xMWXb5zHS4QQ=; b=DQ4V5gYqwPQSE34vO/2DdeROkEIi89HBn347soLU7CllUa6amLfopCqaoJKJSjgFEvhYWJ tSVUxtPZznCGJa18w9euoaLxBfglWaD1rs9GhBtEYujDhbJ5b7Aga2oqTvS2+VkxzM7xB5 3uN5T6S20ZAmzDd4/A/B24MQuxfA67c= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1707209999; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=KyrMNiTZMVYb6KNeedcQB3XxOGdJo71xMWXb5zHS4QQ=; b=lMq8nruIVQE2+TK/dzAcMw1ISqn8sz6M0ORC2RYzMOa9Peifa3XabK8pCX16HmmrQzuRvt 0wmphaZ6rXeyhbBg== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1707209999; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=KyrMNiTZMVYb6KNeedcQB3XxOGdJo71xMWXb5zHS4QQ=; b=DQ4V5gYqwPQSE34vO/2DdeROkEIi89HBn347soLU7CllUa6amLfopCqaoJKJSjgFEvhYWJ tSVUxtPZznCGJa18w9euoaLxBfglWaD1rs9GhBtEYujDhbJ5b7Aga2oqTvS2+VkxzM7xB5 3uN5T6S20ZAmzDd4/A/B24MQuxfA67c= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1707209999; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=KyrMNiTZMVYb6KNeedcQB3XxOGdJo71xMWXb5zHS4QQ=; b=lMq8nruIVQE2+TK/dzAcMw1ISqn8sz6M0ORC2RYzMOa9Peifa3XabK8pCX16HmmrQzuRvt 0wmphaZ6rXeyhbBg== Date: Tue, 6 Feb 2024 09:59:58 +0100 (CET) From: Richard Biener To: Jeff Law cc: gcc-patches@gcc.gnu.org Subject: Re: [PATCH 2/2] rtl-optimization/113255 - avoid re-associating REG_POINTER MINUS In-Reply-To: <5ce7ca28-d22c-43a2-9d92-43a87f3b0304@gmail.com> Message-ID: <23rrs00s-o578-q013-8040-np6n961134o5@fhfr.qr> References: <20240201142121.B149B38582BB@sourceware.org> <2bf60308-cb26-414c-a6e2-4acecf78a53f@gmail.com> <5ce7ca28-d22c-43a2-9d92-43a87f3b0304@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Authentication-Results: smtp-out1.suse.de; none X-Spam-Level: X-Spam-Score: -4.29 X-Spamd-Result: default: False [-4.29 / 50.00]; ARC_NA(0.00)[]; FROM_HAS_DN(0.00)[]; TO_DN_SOME(0.00)[]; FREEMAIL_ENVRCPT(0.00)[gmail.com]; TO_MATCH_ENVRCPT_ALL(0.00)[]; MIME_GOOD(-0.10)[text/plain]; NEURAL_HAM_LONG(-1.00)[-1.000]; DKIM_SIGNED(0.00)[suse.de:s=susede2_rsa,suse.de:s=susede2_ed25519]; NEURAL_HAM_SHORT(-0.19)[-0.967]; RCPT_COUNT_TWO(0.00)[2]; FREEMAIL_TO(0.00)[gmail.com]; RCVD_COUNT_ZERO(0.00)[0]; FROM_EQ_ENVFROM(0.00)[]; MIME_TRACE(0.00)[0:+]; FUZZY_BLOCKED(0.00)[rspamd.com]; BAYES_HAM(-3.00)[100.00%] X-Spam-Status: No, score=-5.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,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 List-Id: On Mon, 5 Feb 2024, Jeff Law wrote: > > > On 2/5/24 01:15, Richard Biener wrote: > > >>> > >>> PR rtl-optimization/113255 > >>> * simplify-rtx.cc (simplify_context::simplify_binary_operation_1): > >>> Do not re-associate a MINUS with a REG_POINTER op0. > >> Nasty little set of problems. I don't think we ever pondered that we could > >> have multiple REGNO_POINTER_FLAG objects in the same expression, but > >> clearly > >> that can happen once you introduce a 3rd term in the expression. > >> > >> I don't mind avoiding the reassociation, but it feels like we're papering > >> over > >> problems in alias.cc. Conceptually it seems like if we have two objects > >> with > >> REG_POINTER set, then we can't know which one is the real base. So your > >> patch > >> in the PR wasn't that bad. > > > > It wasn't bad, it's the only correct fix. The question is what we do > > for branches (or whether we do anything there) and whether we just accept > > that that fix causes some optimization regressions. > For the branches, I'd go whatever you feel the safest change is. While it > looks like some of this is fundamentally broken, it can't be *that* bad since > it's just rearing its ugly head now. It did in the past as well where we worked around by tweaking either code generation or heuristics. > I could even make a case that going with the patch from the PR for the > branches is reasonable. It's attacking at least part of the root problem. > > > > >> Alternately, just stop using REG_POINTER for alias analysis? It looks > >> fundamentally flawed to me in that context. In fact, one might argue that > >> the > >> only legitimate use would be to indicate to the target that we know a > >> pointer > >> points into an object. Some targets (the PA) need this because x + y is > >> not > >> the same as y + x when used as a memory address. > >> > >> If we wanted to be a bit more surgical, drop REG_POINTER from just the > >> MINUS > >> handling in alias.cc? > > > > The problem is that REG_POINTER is just used as a heuristic > > (and compile-time optimization) as to which of a binary operator > > operands we use a base of (preferrably). find_base_{term,value} > > happily look at operands that are not REG_POINTER (that are > > not REG_P), since for the case in question, even w/o re-assoc > > there would be no way to say the inner MINUS is not a pointer > > (it's a REG flag). > > > > The heuristics don't help much when passes like DSE use CSELIB > > and combine operations like above, we then get to see that > > the way find_base_{term,value} perform pointer analysis is > > fundamentally flawed. Any tweaking there has the chance to > > make other cases run into wrong base discoveries. > > > Exactly. So maybe I'm missing something -- it sounds like we both agree that > using REG_POINTER in the aliasing code is just fundamentally broken in the > modern world (and perhaps has been for a long time). So we "just" need to > excise that code from alias.cc. Btw, it's not so much REG_POINTER that is problematic - it's that find_base_{term,value} for binary operators doesn't merge the "points-to solution" for both operands and that if it doesn't find a "base" in the part of the IL it sees it assumes the points-to set is empty. That is, it combines "is not a pointer" and "I have no idea" in the NULL return value. The fix that's now installed on trunk "solves" this lack of merging by never handling any case that would require merging (optimistically treating CONST_INT as known not pointer). I don't think proper PTA analysis on RTL will yield anything useful and we're better off tracking the guarantees which it tries to handle with the ADDRESS base values for stack, spill and argument space when we create MEMs (and annotate MEMs). But I have a hard time deciphering RTL details which isn't really my main area of expertise ... We do already tag some MEMs (like spills with their special MEM_ATTRs), but argument setup seems lacking in this regard, and that's the difficult to understand part since it involves three different unique_base_value (based on STACK_POINTER_REGNUM, ARG_POINTER_REGNUM and FRAME_POINTER_REGNUM) and the base of a MEM seems to vary based on elimination state :/ > > I'll take it that we need to live with the regressions for GCC 14 > > and the wrong-code bug in GCC 13 and earlier. > I'm not sure I agree with this statement. Or maybe I thought the patch in the > PR was more effective than it really is. At some level we ought to be able to > cut out the short-cuts enabled by REG_POINTER. That runs the risk of > perturbing more code, but it seems to me that's a risk we might need to take. REG_POINTER just determines which operand we prefer, so it's a heuristic for the case when multiple bases are involved in the computation of the final value. Richard.