From patchwork Fri Jan 27 12:34:05 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Biener X-Patchwork-Id: 138218 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 B4AAAB6F65 for ; Fri, 27 Jan 2012 23:34:37 +1100 (EST) Comment: DKIM? See http://www.dkim.org DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=gcc.gnu.org; s=default; x=1328272479; h=Comment: DomainKey-Signature:Received:Received:Received:Received:Date: From:To:Cc:Subject:In-Reply-To:Message-ID:References:User-Agent: MIME-Version:Content-Type:Mailing-List:Precedence:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:Sender: Delivered-To; bh=fri92oZxz650cp6Yl2+/+u9Hmmo=; b=k4Ci8Ed74Z0L56H 1VUzwtSDUQ7Fm+yDA3iq54etxKJsVnpyemjAZHlBQ2DGj2Ydh+tl9Po+lJdKmW2e cA4sW7GJkg88k0fZDG0s8EVw+c4pz8wV0JCA/oBIk3zEbNXSJ3Dy5HHUXDV3z/Gd TONxYHEpyyi4Yclge6JnLwONtzS4= Comment: DomainKeys? See http://antispam.yahoo.com/domainkeys DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=default; d=gcc.gnu.org; h=Received:Received:X-SWARE-Spam-Status:X-Spam-Check-By:Received:Received:Date:From:To:Cc:Subject:In-Reply-To:Message-ID:References:User-Agent:MIME-Version:Content-Type:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=qJrgBkbrOPYWFxXRUJsqz5AGVgJXmXD08EkXOrBrZRjxgfqD+p6itpfTqcg9Au xs7/mLnl+uEvF/frVxkZL49C9Yte1sH52JAETCya5HTpqOKvzwEH/C1HYOTf7qYS m0Sfqe0jQwiIAA2r89MWiwL3v3JIfm4PLOrYixhtEeYzQ=; Received: (qmail 30696 invoked by alias); 27 Jan 2012 12:34:27 -0000 Received: (qmail 30274 invoked by uid 22791); 27 Jan 2012 12:34:23 -0000 X-SWARE-Spam-Status: No, hits=-5.7 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_HI, T_RP_MATCHES_RCVD 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; Fri, 27 Jan 2012 12:34:07 +0000 Received: from relay1.suse.de (unknown [195.135.220.254]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx2.suse.de (Postfix) with ESMTP id 76A9F8C061; Fri, 27 Jan 2012 13:34:05 +0100 (CET) Date: Fri, 27 Jan 2012 13:34:05 +0100 (CET) From: Richard Guenther To: Eric Botcazou Cc: Jakub Jelinek , gcc-patches@gcc.gnu.org Subject: Re: [PATCH] Fix expansion of BLKmode MEM_REF with non-addressable non-BLKmode base decl (PR middle-end/51895) In-Reply-To: Message-ID: References: <20120123181259.GK18768@tyan-ft48-01.lab.bos.redhat.com> <201201271109.37706.ebotcazou@adacore.com> User-Agent: Alpine 2.00 (LNX 1167 2008-08-23) 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 On Fri, 27 Jan 2012, Richard Guenther wrote: > On Fri, 27 Jan 2012, Eric Botcazou wrote: > > > > So I am testing the following - please tell me whether you are working > > > on a different fix. > > > > I was, but I realized that this would somewhat conflict with your latest patch > > to expand_assignment, where all MEM_REFs will go through get_inner_reference > > instead of the regular expander. > > > > Can't we avoid doing this if they have BLKmode? Because you cannot get a > > BLKmode RTX out of this if the base hasn't BLKmode. We would need to spill, > > like in the regular expander, so we might as well use it. > > I was simply trying to save some code-duplication here. I will > re-work the patch to avoid this as you suggest. Btw, the original > reason why we handle MEM_REF at all via the get_inner_reference > path is that if we have MEM_REF[&decl, CST] and decl was not > committed to memory then the MEM_REF really acts like a > bitfield insert to a non-MEM (similar to the rvalue case we > handle in expand_expr_real_1). > > I suppose I should split out some predicates that can be shared > amongst the various TARGET_[MEM_REF] handlers during expansion > and tighten this one up as well. > > > Otherwise, you're the resident expert in aliasing so, if you think that your > > patchlet is sufficient, fine with me. > > Yes, I think it is sufficient for this case. > > Now, let me rework that expand_assignment patch a bit. Like the following, bootstrapped and tested on x86_64-unknown-linux-gnu. I'll apply it later unless I hear some objections. Thanks, Richard. 2012-01-27 Richard Guenther PR tree-optimization/50444 * expr.c (mem_ref_refers_to_non_mem_p): New function. (expand_assignment): Use it. Properly handle misaligned bases when expanding stores to component references. (expand_expr_real_1): Use mem_ref_refers_to_non_mem_p and refactor that case. Index: gcc/expr.c =================================================================== --- gcc/expr.c (revision 183616) +++ gcc/expr.c (working copy) @@ -4548,6 +4548,23 @@ get_bit_range (unsigned HOST_WIDE_INT *b } } +/* Returns true if the MEM_REF REF refers to an object that does not + reside in memory and has non-BLKmode. */ + +static bool +mem_ref_refers_to_non_mem_p (tree ref) +{ + tree base = TREE_OPERAND (ref, 0); + if (TREE_CODE (base) != ADDR_EXPR) + return false; + base = TREE_OPERAND (base, 0); + return (DECL_P (base) + && !TREE_ADDRESSABLE (base) + && DECL_MODE (base) != BLKmode + && DECL_RTL_SET_P (base) + && !MEM_P (DECL_RTL (base))); +} + /* Expand an assignment that stores the value of FROM into TO. If NONTEMPORAL is true, try generating a nontemporal store. */ @@ -4571,6 +4588,7 @@ expand_assignment (tree to, tree from, b if (operand_equal_p (to, from, 0)) return; + /* Handle misaligned stores. */ mode = TYPE_MODE (TREE_TYPE (to)); if ((TREE_CODE (to) == MEM_REF || TREE_CODE (to) == TARGET_MEM_REF) @@ -4580,6 +4598,8 @@ expand_assignment (tree to, tree from, b && ((icode = optab_handler (movmisalign_optab, mode)) != CODE_FOR_nothing)) { + addr_space_t as + = TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (TREE_OPERAND (to, 0)))); struct expand_operand ops[2]; enum machine_mode address_mode; rtx reg, op0, mem; @@ -4589,8 +4609,6 @@ expand_assignment (tree to, tree from, b if (TREE_CODE (to) == MEM_REF) { - addr_space_t as - = TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (TREE_OPERAND (to, 0)))); tree base = TREE_OPERAND (to, 0); address_mode = targetm.addr_space.address_mode (as); op0 = expand_expr (base, NULL_RTX, VOIDmode, EXPAND_NORMAL); @@ -4598,7 +4616,7 @@ expand_assignment (tree to, tree from, b if (!integer_zerop (TREE_OPERAND (to, 1))) { rtx off - = immed_double_int_const (mem_ref_offset (to), address_mode); + = immed_double_int_const (mem_ref_offset (to), address_mode); op0 = simplify_gen_binary (PLUS, address_mode, op0, off); } op0 = memory_address_addr_space (mode, op0, as); @@ -4608,10 +4626,7 @@ expand_assignment (tree to, tree from, b } else if (TREE_CODE (to) == TARGET_MEM_REF) { - addr_space_t as - = TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (TREE_OPERAND (to, 0)))); struct mem_address addr; - get_address_description (to, &addr); op0 = addr_for_mem_ref (&addr, as, true); op0 = memory_address_addr_space (mode, op0, as); @@ -4627,7 +4642,7 @@ expand_assignment (tree to, tree from, b create_fixed_operand (&ops[0], mem); create_input_operand (&ops[1], reg, mode); /* The movmisalign pattern cannot fail, else the assignment would - silently be omitted. */ + silently be omitted. */ expand_insn (icode, 2, ops); return; } @@ -4636,12 +4651,10 @@ expand_assignment (tree to, tree from, b if the structure component's rtx is not simply a MEM. Assignment of an array element at a constant index, and assignment of an array element in an unaligned packed structure field, has the same - problem. */ + problem. Same for (partially) storing into a non-memory object. */ if (handled_component_p (to) - /* ??? We only need to handle MEM_REF here if the access is not - a full access of the base object. */ || (TREE_CODE (to) == MEM_REF - && TREE_CODE (TREE_OPERAND (to, 0)) == ADDR_EXPR) + && mem_ref_refers_to_non_mem_p (to)) || TREE_CODE (TREE_TYPE (to)) == ARRAY_TYPE) { enum machine_mode mode1; @@ -4652,6 +4665,7 @@ expand_assignment (tree to, tree from, b int unsignedp; int volatilep = 0; tree tem; + bool misalignp; push_temp_slots (); tem = get_inner_reference (to, &bitsize, &bitpos, &offset, &mode1, @@ -4664,8 +4678,22 @@ expand_assignment (tree to, tree from, b /* If we are going to use store_bit_field and extract_bit_field, make sure to_rtx will be safe for multiple use. */ - - to_rtx = expand_normal (tem); + mode = TYPE_MODE (TREE_TYPE (tem)); + if (TREE_CODE (tem) == MEM_REF + && mode != BLKmode + && ((align = get_object_or_type_alignment (tem)) + < GET_MODE_ALIGNMENT (mode)) + && ((icode = optab_handler (movmisalign_optab, mode)) + != CODE_FOR_nothing)) + { + misalignp = true; + to_rtx = gen_reg_rtx (mode); + } + else + { + misalignp = false; + to_rtx = expand_normal (tem); + } /* If the bitfield is volatile, we want to access it in the field's mode, not the computed mode. @@ -4811,6 +4839,37 @@ expand_assignment (tree to, tree from, b nontemporal); } + if (misalignp) + { + struct expand_operand ops[2]; + enum machine_mode address_mode; + rtx op0, mem; + addr_space_t as = TYPE_ADDR_SPACE + (TREE_TYPE (TREE_TYPE (TREE_OPERAND (tem, 0)))); + tree base = TREE_OPERAND (tem, 0); + address_mode = targetm.addr_space.address_mode (as); + op0 = expand_expr (base, NULL_RTX, VOIDmode, EXPAND_NORMAL); + op0 = convert_memory_address_addr_space (address_mode, op0, as); + if (!integer_zerop (TREE_OPERAND (tem, 1))) + { + rtx off = immed_double_int_const (mem_ref_offset (tem), + address_mode); + op0 = simplify_gen_binary (PLUS, address_mode, op0, off); + } + op0 = memory_address_addr_space (mode, op0, as); + mem = gen_rtx_MEM (mode, op0); + set_mem_attributes (mem, tem, 0); + set_mem_addr_space (mem, as); + if (TREE_THIS_VOLATILE (tem)) + MEM_VOLATILE_P (mem) = 1; + + create_fixed_operand (&ops[0], mem); + create_input_operand (&ops[1], to_rtx, mode); + /* The movmisalign pattern cannot fail, else the assignment + would silently be omitted. */ + expand_insn (icode, 2, ops); + } + if (result) preserve_temp_slots (result); free_temp_slots (); @@ -4866,11 +4925,8 @@ expand_assignment (tree to, tree from, b return; } - /* Ordinary treatment. Expand TO to get a REG or MEM rtx. - Don't re-expand if it was expanded already (in COMPONENT_REF case). */ - - if (to_rtx == 0) - to_rtx = expand_expr (to, NULL_RTX, VOIDmode, EXPAND_WRITE); + /* Ordinary treatment. Expand TO to get a REG or MEM rtx. */ + to_rtx = expand_expr (to, NULL_RTX, VOIDmode, EXPAND_WRITE); /* Don't move directly into a return register. */ if (TREE_CODE (to) == RESULT_DECL @@ -9295,54 +9351,38 @@ expand_expr_real_1 (tree exp, rtx target unsigned align; /* Handle expansion of non-aliased memory with non-BLKmode. That might end up in a register. */ - if (TREE_CODE (base) == ADDR_EXPR) + if (mem_ref_refers_to_non_mem_p (exp)) { HOST_WIDE_INT offset = mem_ref_offset (exp).low; tree bit_offset; + tree bftype; base = TREE_OPERAND (base, 0); - if (!DECL_P (base)) - { - HOST_WIDE_INT off; - base = get_addr_base_and_unit_offset (base, &off); - gcc_assert (base); - offset += off; - } - /* If we are expanding a MEM_REF of a non-BLKmode non-addressable - decl we must use bitfield operations. */ - if (DECL_P (base) - && !TREE_ADDRESSABLE (base) - && DECL_MODE (base) != BLKmode - && DECL_RTL_SET_P (base) - && !MEM_P (DECL_RTL (base))) + if (offset == 0 + && host_integerp (TYPE_SIZE (TREE_TYPE (exp)), 1) + && (GET_MODE_BITSIZE (DECL_MODE (base)) + == TREE_INT_CST_LOW (TYPE_SIZE (TREE_TYPE (exp))))) + return expand_expr (build1 (VIEW_CONVERT_EXPR, + TREE_TYPE (exp), base), + target, tmode, modifier); + bit_offset = bitsize_int (offset * BITS_PER_UNIT); + bftype = TREE_TYPE (base); + if (TYPE_MODE (TREE_TYPE (exp)) != BLKmode) + bftype = TREE_TYPE (exp); + else { - tree bftype; - if (offset == 0 - && host_integerp (TYPE_SIZE (TREE_TYPE (exp)), 1) - && (GET_MODE_BITSIZE (DECL_MODE (base)) - == TREE_INT_CST_LOW (TYPE_SIZE (TREE_TYPE (exp))))) - return expand_expr (build1 (VIEW_CONVERT_EXPR, - TREE_TYPE (exp), base), - target, tmode, modifier); - bit_offset = bitsize_int (offset * BITS_PER_UNIT); - bftype = TREE_TYPE (base); - if (TYPE_MODE (TREE_TYPE (exp)) != BLKmode) - bftype = TREE_TYPE (exp); - else - { - temp = assign_stack_temp (DECL_MODE (base), - GET_MODE_SIZE (DECL_MODE (base)), - 0); - store_expr (base, temp, 0, false); - temp = adjust_address (temp, BLKmode, offset); - set_mem_size (temp, int_size_in_bytes (TREE_TYPE (exp))); - return temp; - } - return expand_expr (build3 (BIT_FIELD_REF, bftype, - base, - TYPE_SIZE (TREE_TYPE (exp)), - bit_offset), - target, tmode, modifier); + temp = assign_stack_temp (DECL_MODE (base), + GET_MODE_SIZE (DECL_MODE (base)), + 0); + store_expr (base, temp, 0, false); + temp = adjust_address (temp, BLKmode, offset); + set_mem_size (temp, int_size_in_bytes (TREE_TYPE (exp))); + return temp; } + return expand_expr (build3 (BIT_FIELD_REF, bftype, + base, + TYPE_SIZE (TREE_TYPE (exp)), + bit_offset), + target, tmode, modifier); } address_mode = targetm.addr_space.address_mode (as); base = TREE_OPERAND (exp, 0);