From patchwork Tue Jun 15 17:59:09 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nathan Froyd X-Patchwork-Id: 55766 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id A6286B7DBC for ; Wed, 16 Jun 2010 03:59:24 +1000 (EST) Received: (qmail 14655 invoked by alias); 15 Jun 2010 17:59:20 -0000 Received: (qmail 14635 invoked by uid 22791); 15 Jun 2010 17:59:17 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=AWL, BAYES_00, T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 15 Jun 2010 17:59:11 +0000 Received: (qmail 31050 invoked from network); 15 Jun 2010 17:59:10 -0000 Received: from unknown (HELO localhost) (froydnj@127.0.0.2) by mail.codesourcery.com with ESMTPA; 15 Jun 2010 17:59:10 -0000 Date: Tue, 15 Jun 2010 10:59:09 -0700 From: Nathan Froyd To: Steve Ellcey Cc: Mark Mitchell , gcc-patches@gcc.gnu.org Subject: Re: [PATCH, c++] rewrite vtable initialization construction to use VECs Message-ID: <20100615175908.GB27105@codesourcery.com> References: <20100603204741.GA19235@codesourcery.com> <201006101705.o5AH5wg18034@lucas.cup.hp.com> <20100615122234.GW27105@codesourcery.com> <1276623054.28515.35.camel@hpsje.cup.hp.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1276623054.28515.35.camel@hpsje.cup.hp.com> User-Agent: Mutt/1.5.17+20080114 (2008-01-14) X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org On Tue, Jun 15, 2010 at 10:30:54AM -0700, Steve Ellcey wrote: > On Tue, 2010-06-15 at 05:22 -0700, Nathan Froyd wrote: > > Index: class.c > > =================================================================== > > --- class.c (revision 160473) > > +++ class.c (working copy) > > @@ -7624,7 +7624,7 @@ build_vtbl_initializer (tree binfo, > > > > for (j = 1; j < TARGET_VTABLE_DATA_ENTRY_DISTANCE; ++j) > > { > > - constructor_elt *f = VEC_index (constructor_elt, *inits, > > + constructor_elt *f = VEC_index (constructor_elt, vid.inits, > > new_position + j); > > f->index = NULL_TREE; > > f->value = build1 (NOP_EXPR, vtable_entry_type, > > Well, the bootstrap worked and I got a compiler built but I get a bunch > of failures during C++ and libstdc++ testing. Does the attached failure > and backtrace give you any ideas on where the problem might be? My C++ runtime-fu is very weak. I guess one of the vtable entries must be NULL when it's not supposed to be. Ah, I think I see the bug. Before the patch, we had: tree cur, *prev; for (prev = &vid.inits; (cur = *prev); prev = &TREE_CHAIN (cur)) { tree add = cur; int i; for (i = 1; i < TARGET_VTABLE_DATA_ENTRY_DISTANCE; ++i) add = tree_cons (NULL_TREE, build1 (NOP_EXPR, vtable_entry_type, null_pointer_node), add); *prev = add; } I think I misread how this loop works: it's adding the padding *before* the entries. But my rewrite added the padding *after* the entries. In pictures, before my patch: +---+---+---+---+ | 0 | A | 0 | B | +---+---+---+---+ after my patch: +---+---+---+---+ | A | 0 | B | 0 | +---+---+---+---+ which would explain why we're seeing null pointers. Would you try the below patch (against mainline, not on top of my previous patch), please? I think that should resolve the problems you're seeing, assuming I didn't botch my arithmetic. -Nathan Index: class.c =================================================================== --- class.c (revision 160473) +++ class.c (working copy) @@ -7618,14 +7618,15 @@ build_vtbl_initializer (tree binfo, ix--) { int j; - int new_position = TARGET_VTABLE_DATA_ENTRY_DISTANCE * ix; + int new_position = (TARGET_VTABLE_DATA_ENTRY_DISTANCE * ix + - (TARGET_VTABLE_DATA_ENTRY_DISTANCE - 1)); VEC_replace (constructor_elt, vid.inits, new_position, e); for (j = 1; j < TARGET_VTABLE_DATA_ENTRY_DISTANCE; ++j) { - constructor_elt *f = VEC_index (constructor_elt, *inits, - new_position + j); + constructor_elt *f = VEC_index (constructor_elt, vid.inits, + new_position - j); f->index = NULL_TREE; f->value = build1 (NOP_EXPR, vtable_entry_type, null_pointer_node);