From patchwork Mon Jul 25 17:52:59 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Jambor X-Patchwork-Id: 106726 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 D2FD8B6F9F for ; Tue, 26 Jul 2011 03:53:21 +1000 (EST) Received: (qmail 12525 invoked by alias); 25 Jul 2011 17:53:19 -0000 Received: (qmail 12517 invoked by uid 22791); 25 Jul 2011 17:53:18 -0000 X-SWARE-Spam-Status: No, hits=-3.4 required=5.0 tests=AWL,BAYES_00 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; Mon, 25 Jul 2011 17:53:01 +0000 Received: from relay1.suse.de (charybdis-ext.suse.de [195.135.221.2]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx2.suse.de (Postfix) with ESMTP id DF19C8F3B5; Mon, 25 Jul 2011 19:52:59 +0200 (CEST) Date: Mon, 25 Jul 2011 19:52:59 +0200 From: Martin Jambor To: Ulrich Weigand Cc: Richard Guenther , GCC Patches Subject: Re: [PATCH, PR 49094] Refrain from creating misaligned accesses in SRA Message-ID: <20110725175259.GC17163@virgil.arch.suse.de> Mail-Followup-To: Ulrich Weigand , Richard Guenther , GCC Patches References: <201107201811.p6KIB6jS009963@d06av02.portsmouth.uk.ibm.com> <20110721094032.GA30670@virgil.arch.suse.de> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20110721094032.GA30670@virgil.arch.suse.de> User-Agent: Mutt/1.5.21 (2010-09-15) 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 Hi, On Thu, Jul 21, 2011 at 11:40:32AM +0200, Martin Jambor wrote: > Hi, > > On Thu, Jul 21, 2011 at 10:34:35AM +0200, Richard Guenther wrote: > > On Wed, 20 Jul 2011, Ulrich Weigand wrote: > > > > > Richard Guenther wrote: > > > > On Tue, Jul 19, 2011 at 8:20 PM, Ulrich Weigand wrote: > > > > > The problem is that in this expression > > > > > disappear = VIEW_CONVERT_EXPR(x_8); > > > > > the rhs is considered unaligned and blocks the SRA transformation. > > > > > > > > > > The check you added for SSA_NAMEs doesn't hit, because the SSA_NAME is > > > > > encapsulated in a VIEW_CONVERT_EXPR. When get_object_alignment is called, > > > > > the VIEW_CONVERT_EXPR is stripped off by get_inner_reference and the > > > > > SSA_NAME appears, but then get_object_alignment doesn't handle it > > > > > and just returns the default alignment of 8 bits. > > > > > > > > > > Maybe get_object_alignment should itself handle SSA_NAMEs? > > > > > > > > But what should it return for a rvalue? There is no "alignment" here. > > > > I think SRA should avoid asking for rvalues. > > > > > > I must admit I do not fully understand what the SRA code is attempting > > > to achieve here ... Could you elaborate on what you mean by "avoid > > > asking for rvalues"? Should the SRA code never check the RHS of an > > > assignment for alignment, only the LHS? Or should it classify the RHS > > > tree according to whether the access is rvalue or lvalue (how would > > > that work?)? > > > > Well, it should only ask for stores / loads. I'm not sure what we'd > > want to return as alignment for an rvalue - MAX_ALIGNMENT? What should > > we return for get_object_alignment of an INTEGER_CST for example? > > > > Yeah, we certainly should not be examining alingment of invariants and > of conversions of ssa names in. As far as rvalues in general are > concerned, I don't really know which gimple predicate that would be. > A comment suggests is_gimple_val but that does not return true for a > VIEW_CONVERT_EXPR of an SSA_NAME and would return true for aggregate > variables (which perhaps would not be a problem, however they do have > an alignment). > > So at the moment I'd go for stripping all conversions from exp before > the first if in tree_non_mode_aligned_mem_p and adding > is_gimple_min_invariant to the condition. Does that make sense? Like this? Ulrich, can you please verify it works? I have bootstrapped this on x86_64 but there it obvioulsy works and my run of compile/testsuite on compile farm sparc64 will take some time (plus, the testcase you complained about passes there). Thanks, Martin 2011-07-25 Martin Jambor * tree-sra.c (tree_non_mode_aligned_mem_p): Strip conversions and return false for invariants. Index: src/gcc/tree-sra.c =================================================================== --- src.orig/gcc/tree-sra.c +++ src/gcc/tree-sra.c @@ -1075,9 +1075,14 @@ tree_non_mode_aligned_mem_p (tree exp) enum machine_mode mode = TYPE_MODE (TREE_TYPE (exp)); unsigned int align; + while (CONVERT_EXPR_P (exp) + || TREE_CODE (exp) == VIEW_CONVERT_EXPR) + exp = TREE_OPERAND (exp, 0); + if (TREE_CODE (exp) == SSA_NAME || TREE_CODE (exp) == MEM_REF || mode == BLKmode + || is_gimple_min_invariant (exp) || !STRICT_ALIGNMENT) return false;