From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15180 invoked by alias); 7 Jun 2013 21:14:44 -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 15169 invoked by uid 89); 7 Jun 2013 21:14:44 -0000 X-Spam-SWARE-Status: No, score=-6.6 required=5.0 tests=AWL,BAYES_00,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.1 Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Fri, 07 Jun 2013 21:14:43 +0000 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r57LEYfF021371 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Fri, 7 Jun 2013 17:14:34 -0400 Received: from zalov.cz (vpn-52-245.rdu2.redhat.com [10.10.52.245]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id r57LEV7R011546 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Fri, 7 Jun 2013 17:14:32 -0400 Received: from zalov.cz (localhost [127.0.0.1]) by zalov.cz (8.14.5/8.14.5) with ESMTP id r57LESh2028132; Fri, 7 Jun 2013 23:14:28 +0200 Received: (from jakub@localhost) by zalov.cz (8.14.5/8.14.5/Submit) id r57LELhC028131; Fri, 7 Jun 2013 23:14:21 +0200 Date: Fri, 07 Jun 2013 21:14:00 -0000 From: Jakub Jelinek To: Richard Henderson Cc: Richard Biener , Jan Hubicka , gcc-patches@gcc.gnu.org, bernds@codesourcery.com, hp@axis.com, hp@bitrange.com, uweigand@de.ibm.com, Andreas.Krebbel@de.ibm.com, dje.gcc@gmail.com Subject: Re: [PATCH] DATA_ALIGNMENT vs. DATA_ABI_ALIGNMENT (PR target/56564) Message-ID: <20130607211419.GI1493@tucnak.redhat.com> Reply-To: Jakub Jelinek References: <20130607192540.GH1493@tucnak.redhat.com> <51B245EF.3080602@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <51B245EF.3080602@redhat.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-SW-Source: 2013-06/txt/msg00435.txt.bz2 On Fri, Jun 07, 2013 at 01:43:27PM -0700, Richard Henderson wrote: > On 06/07/2013 12:25 PM, Jakub Jelinek wrote: > > This PR is about DATA_ALIGNMENT macro increasing alignment of some decls > > for optimization purposes beyond ABI mandated levels. It is fine to emit > > the vars aligned as much as we want for optimization purposes, but if we > > can't be sure that references to that decl bind to the definition we > > increased the alignment on (e.g. common variables, or -fpic code without > > hidden visibility, weak vars etc.), we can't assume that alignment. > > When the linker merges common blocks, it chooses both maximum size and maximum > alignment. Thus for any common block for which we can prove the block must > reside in the module (any executable, or hidden common in shared object), we > can go ahead and use the increased alignment. But consider say: one TU: struct S { char buf[15]; } s __attribute__((aligned (32))); another TU: char c = 7; struct S { char buf[15]; } s = { { 1, 2 } }; char d = 8; int main () { return 0; } (the aligned(32) is there just to simulate the DATA_ALIGNMENT optimization increase). Linker warns about this (thus the question is if we want to increase the alignment for optimization on commons at all) and doesn't align it. > This structure would seem to do the wrong thing if DATA_ABI_ALIGNMENT is > defined, but DATA_ALIGNMENT isn't. And while I realize you documented it, I > don't like the restriction that D_A /must/ return something larger than D_A_A. > All that means is that in complex cases D_A will have to call D_A_A itself. Yeah, I guess I can rearrange it. The reason I wrote it that way was to avoid an extra function call, but that is probably not big enough overhead. > I would think that it would be better to rearrange as > > if (!D_U_A) > { > #ifdef D_A_A > align = ... > #endif > #ifdef D_A > if (d_b_t_c_d_p) > align = ... > #endif > } > > Why the special case for TLS? If we really want that special case surely that > test should go into D_A_A itself, and not here in generic code. I special case TLS for backwards ABI compatibility with GCC <= 4.8.1, because we ignored DATA_ALIGNMENT for TLS vars, even if what it returned wasn't just an optimization, but ABI requirement. So, if you have: __thread char a[16] = { 1, 2 }; in one shared library and __thread char a[16] = { 1, 2 }; use (a); in another shared library, compile one with GCC 4.8.1 e.g. and the other one with 4.9.0, if use (a) would assume the psABI mandated 16 byte alignment, but the actual definition was from 4.8.1 compiled object and was only 1 byte aligned, it could crash. Also, neither DATA_ALIGNMENT nor DATA_ABI_ALIGNMENT sees the decl itself, it just looks at type and previously assumed alignment. > > Bootstrapped/regtested on x86_64-linux and i686-linux. No idea about other > > targets, I've kept them all using DATA_ALIGNMENT, which is considered > > optimization increase only now, if there is some ABI mandated alignment > > increase on other targets, that should be done in DATA_ABI_ALIGNMENT as > > well as DATA_ALIGNMENT. > > I've had a brief look over the instances of D_A within the tree atm. Most of > them carry the cut-n-paste comment "for the same reasons". These I believe > never intended an ABI change, and were really only interested in optimization. Thanks for looking into this. > But these I think require a good hard look to see if they really intended an > ABI alignment: > > c6x comment explicitly mentions abi > cris compiler options for alignment -- systemwide or local? > mmix comment mentions GETA instruction > s390 comment mentions LARL instruction > rs6000 SPE and E500 portion of the alignment non-optional? > > Relevant port maintainers CCed. Jakub