From patchwork Wed Jun 29 08:28:54 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Sandiford X-Patchwork-Id: 102537 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 BE9FEB6F5F for ; Wed, 29 Jun 2011 18:29:19 +1000 (EST) Received: (qmail 5351 invoked by alias); 29 Jun 2011 08:29:17 -0000 Received: (qmail 5341 invoked by uid 22791); 29 Jun 2011 08:29:15 -0000 X-SWARE-Spam-Status: No, hits=-2.4 required=5.0 tests=AWL, BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW X-Spam-Check-By: sourceware.org Received: from mail-fx0-f49.google.com (HELO mail-fx0-f49.google.com) (209.85.161.49) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 29 Jun 2011 08:29:00 +0000 Received: by fxd20 with SMTP id 20so868037fxd.22 for ; Wed, 29 Jun 2011 01:28:59 -0700 (PDT) Received: by 10.223.97.65 with SMTP id k1mr896736fan.0.1309336138998; Wed, 29 Jun 2011 01:28:58 -0700 (PDT) Received: from richards-thinkpad (gbibp9ph1--blueice3n2.emea.ibm.com [195.212.29.84]) by mx.google.com with ESMTPS id j23sm667741fai.39.2011.06.29.01.28.57 (version=TLSv1/SSLv3 cipher=OTHER); Wed, 29 Jun 2011 01:28:58 -0700 (PDT) From: Richard Sandiford To: "H.J. Lu" Mail-Followup-To: "H.J. Lu" , Richard Guenther , gcc-patches@gcc.gnu.org, rdsandiford@googlemail.com Cc: Richard Guenther , gcc-patches@gcc.gnu.org Subject: Re: PR 49169: testing the alignment of a function References: Date: Wed, 29 Jun 2011 09:28:54 +0100 In-Reply-To: (H. J. Lu's message of "Mon, 27 Jun 2011 06:14:50 -0700") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.1 (gnu/linux) MIME-Version: 1.0 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 "H.J. Lu" writes: > This caused: > > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49545 Sorry for the breakage. I should obviously have tested on x86_64 as well. To recap, there are (at least) two concepts of what "the address of X" can mean. It can mean (a) the address at which X is actually located or (b) the value to which symbol X resolves. The problem is that these two are different for things like Thumb and MIPS16 functions. The DECL_ALIGN on a function should be (a). For example, if we have: void __attribute__((aligned(16))) foo (void) { ... } then foo() really ought to start on a 16-byte boundary, even for Thumb. However, most parts of GCC want (b). In particular, if we want to know the alignment of an ADDR_EXPR, or the alignment of a tree or RTL memory reference, the address is going to come from symbol resolution. As far as I can tell, all uses of get_object_alignment* want (b). In particular, get_object_alignment* isn't used for __alignof__, which might have been one case where (a) made more sense. (Any thoughts on what __alignof__ should mean here? Either way, it's a separate patch.) So DECL_ALIGN gives (a) rather than (b). But as the PR shows, there are cases where we know (and need to know) that (b) >= 2 bytes, namely whenever TARGET_PTRMEMFUNC_VBIT_LOCATION == ptrmemfunc_vbit_in_pfn. This patch uses that to decide whether &function has 1 or 2 bytes of alignment. Note that this isn't just about ARM and MIPS. Some targets define FUNCTION_BOUNDARY in a way that depends on optimisation flags, whereas get_object_alignment previously treated it as an ABI property. It's definitely arguable that those targets are buggy and should be using align_functions instead, but the docs aren't really clear. That's why I'm still not going for a target hook at this stage. It's just too hard to say in general whether a port's current FUNCTION_BOUNDARY is guaranteed by the ABI or not, so I think the default definition of any hook would still be the condition that I've used here. I think it makes sense to leave it like this until someone is motivated to guarantee greater alignment. Tested on x86_64-linux-gnu and arm-linux-gnueabi. OK to install? Richard gcc/ PR tree-optimization/49545 * builtins.c (get_object_alignment_1): Update function comment. Do not use DECL_ALIGN for functions, but test TARGET_PTRMEMFUNC_VBIT_LOCATION instead. * fold-const.c (get_pointer_modulus_and_residue): Don't check for functions here. * tree-ssa-ccp.c (get_value_from_alignment): Likewise. gcc/testsuite/ * gcc.dg/torture/pr49169.c: Restrict to ARM and MIPS targets. Index: gcc/builtins.c =================================================================== --- gcc/builtins.c 2011-06-28 14:06:19.000000000 +0100 +++ gcc/builtins.c 2011-06-29 09:07:00.000000000 +0100 @@ -264,8 +264,15 @@ called_as_built_in (tree node) return is_builtin_name (name); } -/* Return the alignment in bits of EXP, an object. - Don't return more than MAX_ALIGN no matter what. */ +/* Compute values M and N such that M divides (address of EXP - N) and + such that N < M. Store N in *BITPOSP and return M. + + Note that the address (and thus the alignment) computed here is based + on the address to which a symbol resolves, whereas DECL_ALIGN is based + on the address at which an object is actually located. These two + addresses are not always the same. For example, on ARM targets, + the address &foo of a Thumb function foo() has the lowest bit set, + whereas foo() itself starts on an even address. */ unsigned int get_object_alignment_1 (tree exp, unsigned HOST_WIDE_INT *bitposp) @@ -287,7 +294,21 @@ get_object_alignment_1 (tree exp, unsign exp = DECL_INITIAL (exp); if (DECL_P (exp) && TREE_CODE (exp) != LABEL_DECL) - align = DECL_ALIGN (exp); + { + if (TREE_CODE (exp) == FUNCTION_DECL) + { + /* Function addresses can encode extra information besides their + alignment. However, if TARGET_PTRMEMFUNC_VBIT_LOCATION + allows the low bit to be used as a virtual bit, we know + that the address itself must be 2-byte aligned. */ + if (TARGET_PTRMEMFUNC_VBIT_LOCATION == ptrmemfunc_vbit_in_pfn) + align = 2 * BITS_PER_UNIT; + else + align = BITS_PER_UNIT; + } + else + align = DECL_ALIGN (exp); + } else if (CONSTANT_CLASS_P (exp)) { align = TYPE_ALIGN (TREE_TYPE (exp)); Index: gcc/fold-const.c =================================================================== --- gcc/fold-const.c 2011-06-28 14:06:19.000000000 +0100 +++ gcc/fold-const.c 2011-06-29 08:59:02.000000000 +0100 @@ -9216,8 +9216,7 @@ get_pointer_modulus_and_residue (tree ex *residue = 0; code = TREE_CODE (expr); - if (code == ADDR_EXPR - && TREE_CODE (TREE_OPERAND (expr, 0)) != FUNCTION_DECL) + if (code == ADDR_EXPR) { unsigned int bitalign; bitalign = get_object_alignment_1 (TREE_OPERAND (expr, 0), residue); Index: gcc/tree-ssa-ccp.c =================================================================== --- gcc/tree-ssa-ccp.c 2011-06-28 14:06:19.000000000 +0100 +++ gcc/tree-ssa-ccp.c 2011-06-29 08:59:02.000000000 +0100 @@ -520,10 +520,6 @@ get_value_from_alignment (tree expr) val = bit_value_binop (PLUS_EXPR, TREE_TYPE (expr), TREE_OPERAND (base, 0), TREE_OPERAND (base, 1)); else if (base - /* ??? While function decls have DECL_ALIGN their addresses - may encode extra information in the lower bits on some - targets (PR47239). Simply punt for function decls for now. */ - && TREE_CODE (base) != FUNCTION_DECL && ((align = get_object_alignment (base, BIGGEST_ALIGNMENT)) > BITS_PER_UNIT)) { Index: gcc/testsuite/gcc.dg/torture/pr49169.c =================================================================== --- gcc/testsuite/gcc.dg/torture/pr49169.c 2011-06-29 08:58:51.000000000 +0100 +++ gcc/testsuite/gcc.dg/torture/pr49169.c 2011-06-29 09:00:41.000000000 +0100 @@ -1,3 +1,5 @@ +/* { dg-do compile { target { arm*-*-* || mips*-*-* } } } */ + #include #include