From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 81440 invoked by alias); 7 Jan 2020 11:38:37 -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 81432 invoked by uid 89); 7 Jan 2020 11:38:37 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-18.5 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_PASS autolearn=ham version=3.3.1 spammy= X-HELO: mx2.suse.de Received: from mx2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 07 Jan 2020 11:38:36 +0000 Received: from relay2.suse.de (unknown [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id A7A85ADC8; Tue, 7 Jan 2020 11:38:33 +0000 (UTC) Date: Tue, 07 Jan 2020 11:38:00 -0000 From: Richard Biener To: Richard Sandiford cc: Andrew Pinski , GCC Patches Subject: Re: [PATCH][RFC] Introduce BIT_FIELD_INSERT In-Reply-To: Message-ID: References: User-Agent: Alpine 2.21 (LSU 202 2017-01-01) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-SW-Source: 2020-01/txt/msg00227.txt.bz2 On Tue, 7 Jan 2020, Richard Sandiford wrote: > Richard Biener writes: > > On Tue, 7 Jan 2020, Andrew Pinski wrote: > > > >> On Mon, Jan 6, 2020 at 11:36 PM Richard Biener wrote: > >> > > >> > On Mon, 16 Dec 2019, Andrew Pinski wrote: > >> > > >> > > On Thu, Nov 15, 2018 at 12:31 AM Richard Biener wrote: > >> > > > > >> > > > On Thu, 15 Nov 2018, Richard Biener wrote: > >> > > > > >> > > > > So - can you fix it please? Also note that the VECTOR_CST case > >> > > > > (as in BIT_FIELD_REF) seems to be inconsistent here and counts > >> > > > > "bits" in a different way? > >> > > > > >> > > > And bonus points for documenting BIT_FIELD_REF and BIT_INSERT_EXPR > >> > > > in generic.texi, together with those "details". > >> > > > >> > > This is the fix: > >> > > diff --git a/gcc/fold-const.c b/gcc/fold-const.c > >> > > index 8e9e299..a919b63 100644 > >> > > --- a/gcc/fold-const.c > >> > > +++ b/gcc/fold-const.c > >> > > @@ -12301,6 +12301,8 @@ fold_ternary_loc (location_t loc, enum > >> > > tree_code code, tree type, > >> > > { > >> > > unsigned HOST_WIDE_INT bitpos = tree_to_uhwi (op2); > >> > > unsigned bitsize = TYPE_PRECISION (TREE_TYPE (arg1)); > >> > > + if (BYTES_BIG_ENDIAN) > >> > > + bitpos = TYPE_PRECISION (type) - bitpos - bitsize; > >> > > wide_int tem = (wi::to_wide (arg0) > >> > > & wi::shifted_mask (bitpos, bitsize, true, > >> > > TYPE_PRECISION (type))); > >> > > >> > I guess you need to guard against BYTES_BIG_ENDIAN != WORDS_BIG_ENDIAN > >> > as well. > >> > >> Yes I will add that check. > >> > >> > Also the above only works reliably for mode-precision > >> > integers? We might want to disallow BIT_FIELD_REF/BIT_INSERT_EXPR > >> > on non-mode-precision entities in the GIMPLE/GENERIC verifiers. > >> > >> You added that check already for gimple in r268332 due to PR88739. > >> BIT_FIELD_REF around tree-cfg.c:3083 > >> BIT_INSERT_EXPR around tree-cfg.c:4324 > > > > Ah, good ;) Note neither BIT_FIELD_REF nor BIT_INSERT_EXPR are > > documented in generic.texi and BIT_FIELD_REF is documented in tree.def > > as operating on structs/unions (well, memory). And for register args > > we interpret it as storing the register to memory and interpreting > > the bit positions in memory bit terms (with the store doing endian > > fiddling). > > Ah, was going to ask what the semantics were. :-) That sounds good > because it's essentially the same as for non-paradoxical subregs. > We have routines like subreg_lsb_1 and subreg_offset_from_lsb that > convert byte offsets to shift amounts, so maybe we should move them > to code that's common to both gimple and rtl. The BYTES_BIG_ENDIAN != > WORDS_BIG_ENDIAN calculation is quite subtle, so I don't think we should > open-code it everwhere we need it. > > What about the subbyte part of the bit value? Does that always count > from the lsb of the containing byte? E.g. for the four bytes: > > 0x12, 0x34, 0x56, 0x78 > > what does bit offset == 3, bit size == 7 mean for big-endian? > > > But for vector (register only?) accesses we interpret > > it as specifying lane numbers but at least BIT_FIELD_REF verifying > > doesn't barf on bit/sizes not corresponding to exact vector lanes > > (and I know we introduce non-matching ones via at least VIEW_CONVERT > > "merging" into BIT_FIELD_REFs). > > GCC's vector lane numbering is equivalent to array index numbering for > all endiannesses, so these cases should still be ok for BYTES_BIG_ENDIAN > == WORDS_BIG_ENDIAN, at least on byte boundaries. Not sure about > subbyte boundaries -- guess it depends on the answer to the question > above. I was thinking about, say a SImode extract at offset == 16, size == 32 of a V4SImode vector. Is that to be interpreted as some weird shifted vector lane or as a memory "bit" location after storing the vector to memory? The issue I see here is that once RTL expansion decides to spill and interpret the offset/size in non-lane terms will there ever be a mismatch between both? > For BYTES_BIG_ENDIAN != WORDS_BIG_ENDIAN, any sequence that crosses > a word boundary can lead to ranges that aren't contiguous in registers, > unless the range starts and ends on a word boundary. This would include > some of those vector cases, but it could also include bitfield references > to multiword integers. > > Subregs that cross a word boundary must start and end on a word boundary, > but I guess that would be too draconian for gimple. Yeah. Richard.