From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21710 invoked by alias); 30 Aug 2011 18:13:51 -0000 Received: (qmail 21702 invoked by uid 22791); 30 Aug 2011 18:13:50 -0000 X-SWARE-Spam-Status: No, hits=-6.6 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,SPF_HELO_PASS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 30 Aug 2011 18:13:33 +0000 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p7UIDU0k016559 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 30 Aug 2011 14:13:30 -0400 Received: from houston.quesejoda.com (vpn-231-203.phx2.redhat.com [10.3.231.203]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id p7UIDUU8028054; Tue, 30 Aug 2011 14:13:30 -0400 Message-ID: <4E5D284A.7080006@redhat.com> Date: Tue, 30 Aug 2011 21:33:00 -0000 From: Aldy Hernandez User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:5.0) Gecko/20110707 Thunderbird/5.0 MIME-Version: 1.0 To: Richard Guenther CC: gcc-patches Subject: Re: [C++0x] contiguous bitfields race implementation References: <4DC8155A.3040401@redhat.com> <4DDE99D2.4020502@redhat.com> <4DDE9DED.6040801@redhat.com> <4DDFF90E.7070408@redhat.com> <4E2420E6.8090809@redhat.com> <4E29C502.8020100@redhat.com> <4E2DA2BA.1010003@redhat.com> <4E2E0264.30909@redhat.com> <4E2EED10.5030901@redhat.com> <4E2EF1E7.4020606@redhat.com> <4E2EFA1C.10803@redhat.com> <4E2EFB89.7060503@redhat.com> <4E3C2773.30502@redhat.com> <4E417EE4.9040501@redhat.com> <4E495D11.5000306@redhat.com> <4E57EBDC.9090900@redhat.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit 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: 2011-08/txt/msg02490.txt.bz2 > Btw, *byte_offset is still not relative to the containing object as > documented, but relative to the base object of the exp reference > tree (thus, to a in a.i.j.k.l instead of to a.i.j.k). If it were supposed > to be relative to a.i.j.k get_inner_reference would be not needed > either. Can you clarify what "containing object" means in the > overall comment please? I'm thoroughly confused here. Originally I had "inner decl", then we changed the nomenclature to "containing object", and now there's this "innermost reference". What I mean to say is the "a" in a.i.j.k.l. How would you like me to call that? The innermost reference? The inner decl? Would this comment be acceptable: Given a COMPONENT_REF, this function calculates the byte offset from the innermost reference ("a" in a.i.j.k.l) to the start of the contiguous bit region containing the field in question. > > If it is really relative to the innermost reference of exp you can > "CSE" the offset of TREE_OPERAND (exp, 0) and do relative > adjustments for all the other get_inner_reference calls. For > example the > > + /* If we found the end of the bit field sequence, include the > + padding up to the next field... */ > if (fld) > { > ... > + /* Calculate bitpos and offset of the next field. */ > + get_inner_reference (build3 (COMPONENT_REF, > + TREE_TYPE (exp), > + TREE_OPERAND (exp, 0), > + fld, NULL_TREE), > + &tbitsize,&end_bitpos,&end_offset, > + &tmode,&tunsignedp,&tvolatilep, true); > > case is not correct anyway, fld may have variable position > (non-INTEGER_CST DECL_FIELD_OFFSET), you can't > assume Innermost here means "a" in a.i.j.k.l? If so, this is what we're currently doing, *byte_offset is the start of the bit region, and *bit_offset is the offset from that. First, I thought we couldn't get a variable position here because we are now handling that case at the beginning of the function with: /* Be as conservative as possible on variable offsets. */ if (TREE_OPERAND (exp, 2) && !host_integerp (TREE_OPERAND (exp, 2), 1)) { *byte_offset = TREE_OPERAND (exp, 2); *maxbits = BITS_PER_UNIT; *bit_offset = 0; return; } And even if we do get a variable position, I have so far being able to get away with this... > > + *maxbits = TREE_INT_CST_LOW (maxbits_tree); > > this thus. ...because the call to fold_build2 immediately preceding this will fold away the variable offset. Is what you want, that we call get_inner_reference once, and then use DECL_FIELD_OFFSET+DECL_FIELD_BIT_OFFSET to calculate any subsequent bit offset? I found this to be quite tricky with padding, and such, but am willing to give it a whirl again. However, could I beg you to reconsider this, and get something working first, only later concentrating on removing the get_inner_reference() calls, and performing any other tweaks/optimizations? Aldy