From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 94824 invoked by alias); 8 Jul 2019 09:43:31 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 94814 invoked by uid 89); 8 Jul 2019 09:43:31 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-5.0 required=5.0 tests=AWL,BAYES_00 autolearn=ham version=3.3.1 spammy=REF, partly X-HELO: nikam.ms.mff.cuni.cz Received: from nikam.ms.mff.cuni.cz (HELO nikam.ms.mff.cuni.cz) (195.113.20.16) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 08 Jul 2019 09:43:29 +0000 Received: by nikam.ms.mff.cuni.cz (Postfix, from userid 16202) id 5F4F7281EDC; Mon, 8 Jul 2019 11:43:27 +0200 (CEST) Date: Mon, 08 Jul 2019 10:48:00 -0000 From: Jan Hubicka To: Richard Biener Cc: gcc-patches@gcc.gnu.org, d@dcepelik.cz Subject: Re: Make nonoverlapping_component_refs work with duplicated main variants Message-ID: <20190708094327.72wcfah7ezh5pxh5@kam.mff.cuni.cz> References: <20190708072649.vqd5u6jxsz5ybtt7@kam.mff.cuni.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: NeoMutt/20170113 (1.7.2) X-SW-Source: 2019-07/txt/msg00560.txt.bz2 > > +/* FIELD1 and FIELD2 are two component refs whose bases are either > > + both at the same address or completely disjoint. > > + Return 1 if FIELD1 and FIELD2 are non-overlapping > > + Return 0 if FIELD1 and FIELD2 are having same addresses or are > > + completely disjoint. > > completely disjoint? I guess > > Return 0 if accesses to FIELD1 and FIELD2 are possibly overlapping. > > is better matching actual behavior. Likewise mentioning 'accesses' > in the first because of the bitfield treatment (the fields may > be non-overlapping but actual accesses might be). I was trying to describe difference between 0 and -1. We return 0 when we fully structurally matched the path and we know it is same. -1 means that we arrived to somehting we can not handle (union, mismatched offsets) and it would make sense to try disambiguating further. Currently it means that in addition to nonoverlapping_component_refs_since_match_p we also do nonoverlapping_component_refs_p which has some chance to recover from the mismatched REF pair, match the types later on path and still disambiguate. It seem to happen very rarely though. > > > + /* Different fields of the same record type cannot overlap. > > + ??? Bitfields can overlap at RTL level so punt on them. */ > > + if (DECL_BIT_FIELD (field1) && DECL_BIT_FIELD (field2)) > > + return -1; > > This is similar as the DECL_BIT_FIELD_REPRESENTATIVE check so why > return -1 instead of 0? Well, my plan is to put this test before ref_and_offset which still have chace to suceed if fields are far away. But i am happy to return 0 here and mess with that later. > > + else > > + { > > + if (operand_equal_p (DECL_FIELD_BIT_OFFSET (field1), > > + DECL_FIELD_BIT_OFFSET (field2), 0)) > > + return 0; > > I think this is overly pessimistic - the offset of a field > is DECL_FIELD_OFFSET + DECL_FIELD_BIT_OFFSET (the latter is > only up to DECL_OFFSET_ALIGN, the rest of the constant > offset spills into DECL_FIELD_OFFSET). Which also means ... > > > + > > + /* Different fields of the same record type cannot overlap. > > + ??? Bitfields can overlap at RTL level so punt on them. */ > > + if (DECL_BIT_FIELD (field1) && DECL_BIT_FIELD (field2)) > > + return -1; > > + > > + poly_uint64 offset1; > > + poly_uint64 offset2; > > + poly_uint64 size1; > > + poly_uint64 size2; > > + if (!poly_int_tree_p (DECL_FIELD_BIT_OFFSET (field1), &offset1) > > + || !poly_int_tree_p (DECL_FIELD_BIT_OFFSET (field2), &offset2) > > + || !poly_int_tree_p (DECL_SIZE (field1), &size1) > > + || !poly_int_tree_p (DECL_SIZE (field2), &size2) > > + || ranges_maybe_overlap_p (offset1, size1, offset2, size2)) > > this is technically wrong in case we had DECL_FIELD_OFFSETs 4 and 8 > and DECL_FIELD_BIT_OFFSETs 32 and 0. > > So you have to compute the combined offsets first. OK, I guess I can take look at the get_base_ref_and_offset there. Thanks for pointing this out. > > > + return -1; > > I think it may make sense to return -1 if any of the !poly_int_tree_p > tests fire, but otherwise? I'm not actually sure what -1 vs. 0 > means here - is 0 a must exactly overlap and -1 is a may overlap > somehow? Well, we have two fields that overlap partly from two different types in >nonoverlapping_component_refs_since_match_p so it can not continue walking (since the main invariant is broken) we may still suceed with the nonoverlaping_component_refs Thanks, I will update the patch. Honza