From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from nikam.ms.mff.cuni.cz (nikam.ms.mff.cuni.cz [195.113.20.16]) by sourceware.org (Postfix) with ESMTPS id 019B73857731; Wed, 14 Feb 2024 15:07:18 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 019B73857731 Authentication-Results: sourceware.org; dmarc=fail (p=none dis=none) header.from=ucw.cz Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=kam.mff.cuni.cz ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 019B73857731 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=195.113.20.16 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1707923241; cv=none; b=oQ7AsZzxRHgpTN/tIme29wwWtvG/tv06Iqeb/y+7xc4SliUaBwP4QlFQdX99m6pm5CN4MlScqvZmrFj5eQeSKXuQHAU9q3EYgcrx7pYXDBKsq74C6bVKYfRvw4Xcy2VtcEPmsl2Ev1jubhet5w9ebe051fhwx7FBRE3NUkjqQOo= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1707923241; c=relaxed/simple; bh=ajUIMBAe8w7as8GE4TfrXrchqR3CIbzqKRfodlH1B/M=; h=DKIM-Signature:Date:From:To:Subject:Message-ID:MIME-Version; b=VzM8HiPPgovqVZLVr7xJuXqikkPfRbda8xzFchJnF2nvuMH0Rw1a2GU30Tn1H6qB6u2keRZJ9MD3mS2D+mc8d2uuQJRGpb3FRR6kPs3j5trF9oJn2915gTZZvkLJRECnJqiRnaBHZ218ZE3tPh+kUZ8boFUZbXhiR7rsElZUksg= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by nikam.ms.mff.cuni.cz (Postfix, from userid 16202) id 96BD0281C72; Wed, 14 Feb 2024 16:07:17 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ucw.cz; s=gen1; t=1707923237; 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: in-reply-to:in-reply-to:references:references; bh=BEwf8mkHOi91dqlxAx3zr9N7SxEiJE1KAwhKBh62B80=; b=crPcFDVCD5Bj/k+TDVtAAFn++6rRPxVM6mKfGOwH5YkFFyoLonQGaXwgnH67fu4CVUbA0U C8Zb8/clZaLhQeGJEu+dqaJPpCCpoEWL9ln9e+U09t1b1c/apN8/9OOyFYyFZDs8/GaevC FIkkC4UNznAiycjBKKCJqdVcwftbQC8= Date: Wed, 14 Feb 2024 16:07:17 +0100 From: Jan Hubicka To: rguenther at suse dot de Cc: gcc-bugs@gcc.gnu.org Subject: Re: [Bug tree-optimization/113787] [12/13/14 Regression] Wrong code at -O with ipa-modref on aarch64 Message-ID: References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Spam-Status: No, score=-10.6 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,GIT_PATCH_0,HEADER_FROM_DIFFERENT_DOMAINS,JMQ_SPF_NEUTRAL,KAM_NUMSUBJECT,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,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: > > I guess PTA gets around by tracking points-to set also for non-pointer > > types and consequently it also gives up on any such addition. > > It does. But note it does _not_ for POINTER_PLUS where it treats > the offset operand as non-pointer. > > > I think it is ipa-prop.c::unadjusted_ptr_and_unit_offset. It accepts > > pointer_plus expression, but does not look through POINTER_PLUS. > > We can restrict it further, but tracking base pointer is quite useful, > > so it would be nice to not give up completely. > > It looks like that function might treat that > > ADDR_EXPR > > > as integer_zerop base. It does > > if (TREE_CODE (op) == ADDR_EXPR) > { > poly_int64 extra_offset = 0; > tree base = get_addr_base_and_unit_offset (TREE_OPERAND (op, 0), > &offset); > if (!base) > { > base = get_base_address (TREE_OPERAND (op, 0)); > if (TREE_CODE (base) != MEM_REF) > break; > offset_known = false; > } > else > { > if (TREE_CODE (base) != MEM_REF) > break; > > with a variable offset we fall to the TREE_CODE (base) != MEM_REF > and will have offset_known == true. Not sure what it does with > the result though (it's not the address of a decl). > > This function seems to oddly special-case != MEM_REF ... (maybe > it wants to hande DECL_P () as finishing? Hmm the function was definitely not written with TARGET_MEM_REF in mind, since it was originally used for IPA passes only. We basically want to handle stuff like &decl->foo or &(ptr->foo) In the second case we want to continue the SSA walk to hopefully work out the origin of PTR. ipa-modref then looks if the base pointer is derived from function parameter or points to local or readonly memory to produce its summary. > > Note get_addr_base_and_unit_offset will return NULL for > a TARGET_MEM_REF <&decl, ..., offset> but TARGET_MEM_REF > itself if the base isn't an ADDR_EXPR, irrespective of whether > the offset within it is constant or not. Hmm, interesting. I would expect it to interpret the emantics of TMR and return base. > > Not sure if the above is a problem, but it seems the only caller > will just call points_to_local_or_readonly_memory_p on the > ADDR_EXPR where refs_local_or_readonly_memory_p via > points_to_local_or_readonly_memory_p will eventually do > > /* See if memory location is clearly invalid. */ > if (integer_zerop (t)) > return flag_delete_null_pointer_checks; > > and that might be a problem. As said, we rely on > ADDR_EXPR > to be an address computation > that's not subject to strict interpretation to allow IVOPTs > doing this kind of optimization w/o introducing some kind of > INTEGER_LEA <...>. I know that's a bit awkward but we should > make sure this is honored by IPA as well. > > I'd say > > diff --git a/gcc/ipa-fnsummary.cc b/gcc/ipa-fnsummary.cc > index 74c9b4e1d1e..45a770cf940 100644 > --- a/gcc/ipa-fnsummary.cc > +++ b/gcc/ipa-fnsummary.cc > @@ -2642,7 +2642,8 @@ points_to_local_or_readonly_memory_p (tree t) > return true; > return !ptr_deref_may_alias_global_p (t, false); > } > - if (TREE_CODE (t) == ADDR_EXPR) > + if (TREE_CODE (t) == ADDR_EXPR > + && TREE_CODE (TREE_OPERAND (t, 0)) != TARGET_MEM_REF) > return refs_local_or_readonly_memory_p (TREE_OPERAND (t, 0)); > return false; > } > > might eventually work? Alternatively a bit less aggressive like > the following. > > diff --git a/gcc/ipa-fnsummary.cc b/gcc/ipa-fnsummary.cc > index 74c9b4e1d1e..7c79adf6440 100644 > --- a/gcc/ipa-fnsummary.cc > +++ b/gcc/ipa-fnsummary.cc > @@ -2642,7 +2642,9 @@ points_to_local_or_readonly_memory_p (tree t) > return true; > return !ptr_deref_may_alias_global_p (t, false); > } > - if (TREE_CODE (t) == ADDR_EXPR) > + if (TREE_CODE (t) == ADDR_EXPR > + && (TREE_CODE (TREE_OPERAND (t, 0)) != TARGET_MEM_REF > + || TREE_CODE (TREE_OPERAND (TREE_OPERAND (t, 0), 0)) != > INTEGER_CST)) > return refs_local_or_readonly_memory_p (TREE_OPERAND (t, 0)); > return false; > } Yes, those both looks reasonable to me, perhaps less agressive would be better. > > A "nicer" solution might be to add a informational operand > to TARGET_MEM_REF, representing the base pointer to be used for > alias/points-to purposes. But if that's not invariant it might > keep some otherwise unnecessary definition stmts live. Yep, I see that forcing extra IV to track original semantics would be trouble here. I think that after iv-opts we should be done with more fancy propagation across loops. However, to avoid ipa-modref summary degradation, perhaps scheduling the pass before ivopts would make sense... Thanks, Honza