From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 51379 invoked by alias); 7 Jan 2020 11:14:46 -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 51369 invoked by uid 89); 7 Jan 2020 11:14:46 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-17.4 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 spammy= X-HELO: foss.arm.com Received: from foss.arm.com (HELO foss.arm.com) (217.140.110.172) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 07 Jan 2020 11:14:42 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id D2A09328; Tue, 7 Jan 2020 03:14:40 -0800 (PST) Received: from localhost (e121540-lin.manchester.arm.com [10.32.98.126]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 430453F534; Tue, 7 Jan 2020 03:14:40 -0800 (PST) From: Richard Sandiford To: Richard Biener Mail-Followup-To: Richard Biener ,Andrew Pinski , GCC Patches , richard.sandiford@arm.com Cc: Andrew Pinski , GCC Patches Subject: Re: [PATCH][RFC] Introduce BIT_FIELD_INSERT References: Date: Tue, 07 Jan 2020 11:14:00 -0000 In-Reply-To: (Richard Biener's message of "Tue, 7 Jan 2020 11:04:00 +0100 (CET)") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-IsSubscribed: yes X-SW-Source: 2020-01/txt/msg00223.txt.bz2 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. 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. Thanks, Richard