From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19997 invoked by alias); 22 Mar 2012 12:39:48 -0000 Received: (qmail 19987 invoked by uid 22791); 22 Mar 2012 12:39:47 -0000 X-SWARE-Spam-Status: No, hits=-5.8 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from cantor2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 22 Mar 2012 12:39:28 +0000 Received: from relay1.suse.de (unknown [195.135.220.254]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx2.suse.de (Postfix) with ESMTP id 58CDF90F2E; Thu, 22 Mar 2012 13:39:27 +0100 (CET) Date: Thu, 22 Mar 2012 12:39:00 -0000 From: Richard Guenther To: Eric Botcazou Cc: Richard Guenther , gcc-patches@gcc.gnu.org Subject: Re: [PATCH] Fix PRs 52080, 52097 and 48124, rewrite bitfield expansion, enable the C++ memory model wrt bitfields everywhere In-Reply-To: <201203221318.47180.ebotcazou@adacore.com> Message-ID: References: <201203221247.42692.ebotcazou@adacore.com> <201203221318.47180.ebotcazou@adacore.com> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII 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 X-SW-Source: 2012-03/txt/msg01499.txt.bz2 On Thu, 22 Mar 2012, Eric Botcazou wrote: > > bitregion_start == 11 looks bogus. The representative is starting at > > > > DECL_FIELD_BIT_OFFSET (repr) > > = size_binop (BIT_AND_EXPR, > > DECL_FIELD_BIT_OFFSET (field), > > bitsize_int (~(BITS_PER_UNIT - 1))); > > > > which looks ok > > It cannot be OK if you want it to be on a byte boundary, since the field isn't > on a byte boundary itself and they have the same DECL_FIELD_BIT_OFFSET (0). Huh? If they have DECL_FIELD_BIT_OFFSET of zero they are at a byte boundary, no? Wait - the RECORD_TYPE itself is at non-zero DECL_FIELD_BIT_OFFSET and thus a zero DECL_FIELD_BIT_OFFSET for its fields does not mean anything?! But how can DECL_OFFSET_ALIGN be still valid for such field? Obviously if DECL_FIELD_OFFSET == 0, DECL_FIELD_BIT_OFFSET == 0 then the offset needs to be aligned to DECL_OFFSET_ALIGN. Which then means DECL_OFFSET_ALIGN is a bit-alignment? Anyway, since we are trying to compute a nice mode to use for the bitfield representative we can give up in the second that we do not know how to reach BITS_PER_UNIT alignment. Or we can simply only try to ensure MIN (BITS_PER_UNIT, DECL_OFFSET_ALIGN) alignment/size of the representative. Of course the bitfield expansion code has to deal with non-byte-aligned representatives then, and we'd always have to use BLKmode for them. > > the size of the representative is (at minimum) > > > > size = size_diffop (DECL_FIELD_OFFSET (field), > > DECL_FIELD_OFFSET (repr)); > > gcc_assert (host_integerp (size, 1)); > > bitsize = (tree_low_cst (size, 1) * BITS_PER_UNIT > > + tree_low_cst (DECL_FIELD_BIT_OFFSET (field), 1) > > - tree_low_cst (DECL_FIELD_BIT_OFFSET (repr), 1) > > + tree_low_cst (DECL_SIZE (field), 1)); > > > > /* Round up bitsize to multiples of BITS_PER_UNIT. */ > > bitsize = (bitsize + BITS_PER_UNIT - 1) & ~(BITS_PER_UNIT - 1); > > > > that looks ok to me as well. Is the issue that we, in get_bit_range, > > compute bitregion_start relative to the byte-aligned offset of the > > representative? > > The issue is that the representative is assumed to be on a byte boundary in > get_bit_range, but it isn't in the case at hand. So either we cope with that > (this is the GCC 4.7 approach) or we change the representative somehow. I think we can't change it to be on a byte-boundary, the same record may be used at different bit-positions, no? > IOW, either we pretend that a bitfield group is entirely contained within a > single record type or we acknowledge that a bitfield group can cross a record > boundary. Sure, we acknowledge it can cross a record boundary. I just was not aware we cannot statically compute the bit-offset to the previous byte for a record type. Richard.