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 91B193858CDB for ; Mon, 5 Feb 2024 08:15:16 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 91B193858CDB 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 91B193858CDB 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=1707120921; cv=none; b=dFZEcXgBUhRzFWrsgVluA4IrH+N3K2nUaZXYAih/+lAJfij0jC6BiTGbcHtSZ70YTxEL7Reze4g6sXa4H0IdNOZADnrOV2LUlTww9zXZjWUGx3x4goWoysWXUUZO1ETIldNB11J2Cln1YkS8x1ncVYCJcvA5xM8SXr0oMSS0u1s= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1707120921; c=relaxed/simple; bh=I6YVJumX5DOi2G2votHNHvuEemEleQXIEUPSTSw18JI=; h=DKIM-Signature:DKIM-Signature:DKIM-Signature:DKIM-Signature:Date: From:To:Subject:Message-ID:MIME-Version; b=o5ZlNr089fHE8F0byXADWPlhoejJBXnkGmkLSbdxGLwkl+5J6X/cQf9/0OkUXsQK/mylTth9X0R9LTxe/3ruYGI0wLvdCcz5yHQlE/YAxs922usRZAp0XDEupJH0+FnFzNc0/Tb0ogtjjKpz/j2QtivwCb/J5QU8BBKmm4xYXyk= 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 F25E322053; Mon, 5 Feb 2024 08:15:13 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1707120915; 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=0CDrTskRKIxv0rqkOARy2Yc4ZnEOhllv0mIZJ67HxBk=; b=HMzp1y/R/fpDDzE1CZnKoCieOTkgvyQUQfJPdrF0iuqFd3DP9+1LgdbxAw5e+7uTDUnbQw NxG8Ck+uM0GBWjxYM0KZcPx1zIbaUjO3EgIfou+yLo88quyNapmlFc/itYJYEHL1FEgwMQ GGvNzFxCa9zl/eNv8ZLY4x+zFtQsU4Q= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1707120915; 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=0CDrTskRKIxv0rqkOARy2Yc4ZnEOhllv0mIZJ67HxBk=; b=1PqZxNnOqfrjwm/Qof6hQ04iARlD69jclfwSPyL4e647uw3pcbUMrNhKT9vQnR9wIAg3s7 VQ9CMUo+8ay08TDw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1707120914; 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=0CDrTskRKIxv0rqkOARy2Yc4ZnEOhllv0mIZJ67HxBk=; b=yFXuDchbUIbJ8IULrLZ8htdGuxa0DWfEK8S8QqQGNi98sIyRZgLbJsCK2yCAmn3Q1U8v8C JsLLMx9P56F7OZOyaHgGTzLKYTBxBVwWY1Vaabx0L4XVqMtwpcS+Td3Nt7FaTGG+kuBJ/L dVCpNFskaHJ/8MHzdJMzzTd6/iwyxEM= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1707120914; 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=0CDrTskRKIxv0rqkOARy2Yc4ZnEOhllv0mIZJ67HxBk=; b=C6M8bR9W2FIQbnaIiC4DdHpb7ulyC2wV8THtkg9STtQ/JM0mgQGyy4ubnEqrVtjRhUspom bK12+YgTRpMfcnAA== Date: Mon, 5 Feb 2024 09:15:13 +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: <2bf60308-cb26-414c-a6e2-4acecf78a53f@gmail.com> Message-ID: References: <20240201142121.B149B38582BB@sourceware.org> <2bf60308-cb26-414c-a6e2-4acecf78a53f@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.969]; RCPT_COUNT_TWO(0.00)[2]; DBL_BLOCKED_OPENRESOLVER(0.00)[alias.cc:url,simplify-rtx.cc:url]; 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 Fri, 2 Feb 2024, Jeff Law wrote: > > > On 2/1/24 07:20, Richard Biener wrote: > > The following avoids re-associating > > > > (minus:DI (reg/f:DI 119) > > (minus:DI (reg/f:DI 120) > > (reg/f:DI 114))) > > > > into > > > > (minus:DI (plus:DI (reg/f:DI 114) > > (reg/f:DI 119)) > > (reg/f:DI 120)) > > > > as that possibly confuses the REG_POINTER heuristics of RTL > > alias analysis. This happens to miscompile the PRs testcase > > during DSE which expands addresses via CSELIB which eventually > > simplifies what it substituted to. The original code does > > the innocent ptr - (ptr2 - ptr2'), bias a pointer by the > > difference of two other pointers. > > > > -- > > > > This is what I propose for the PR for branches, I have not made much > > progress with fixing the fallout on the RTL alias analysis change > > on trunk, so this is the alternative if we decide to revert that. > > > > Bootstrapped and tested on x86_64-unknown-linux-gnu on the gcc-13 > > branch, bootstrapped after reverting of the previous fix on > > x86_64-unknown-linux-gnu on trunk, testing is still ongoing there. > > > > OK? Any preference for trunk? > > > > Thanks, > > Richard. > > > > 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. > 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. 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. Thanks, Richard.