From patchwork Fri Jul 18 08:08:44 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Sandiford X-Patchwork-Id: 371357 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]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4C0C6140078 for ; Fri, 18 Jul 2014 18:09:02 +1000 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:date:message-id:mime-version:content-type; q=dns; s=default; b=nsSInJ3M8bRflgTSwEa4UThbqZJc2RTEeCePyfpTl2pjvxATJX Z0DgzPOFPk0dnOCzci3RcVFVDyNVqAoYMe4w0RYcEDv0s6v+Vhoa0zBb/eops9vT BXIcwfpn1j9Dk8Hu5bCKR5AM0bgI95fMkXp3XoV+A8XtouAZnn8sw/Ll4= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:date:message-id:mime-version:content-type; s= default; bh=S8mPK3HJOf2BmdBxhrPTWrOTpaQ=; b=S1e01TEOo9vP+qlvDnqL I12voyQjJ6gBtBonkFTGWesO2Qx4hcMtjav1K5nQIva+xKFAnmt9h/Itj8bBDnQf UpU7zDNmAu8xXRmkCsnUAvzrzll7Zo8xyyt/am2uDdZzEFgrOQdhm4sdCVnPGVG5 ybZuBukngiSVy3mgjICPLyc= Received: (qmail 25650 invoked by alias); 18 Jul 2014 08:08:52 -0000 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 Received: (qmail 25629 invoked by uid 89); 18 Jul 2014 08:08:51 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.6 required=5.0 tests=BAYES_00, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-wi0-f170.google.com Received: from mail-wi0-f170.google.com (HELO mail-wi0-f170.google.com) (209.85.212.170) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Fri, 18 Jul 2014 08:08:49 +0000 Received: by mail-wi0-f170.google.com with SMTP id f8so750256wiw.5 for ; Fri, 18 Jul 2014 01:08:46 -0700 (PDT) X-Received: by 10.194.243.200 with SMTP id xa8mr3878291wjc.97.1405670926165; Fri, 18 Jul 2014 01:08:46 -0700 (PDT) Received: from localhost ([95.145.138.241]) by mx.google.com with ESMTPSA id pj6sm12304710wjb.21.2014.07.18.01.08.45 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 18 Jul 2014 01:08:45 -0700 (PDT) From: Richard Sandiford To: gcc-patches@gcc.gnu.org Mail-Followup-To: gcc-patches@gcc.gnu.org, ro@CeBiTec.Uni-Bielefeld.DE, rdsandiford@googlemail.com Cc: ro@CeBiTec.Uni-Bielefeld.DE Subject: PR 61628: Invalid sharing of DECL_INCOMING_RTL Date: Fri, 18 Jul 2014 09:08:44 +0100 Message-ID: <871ttjozqr.fsf@talisman.default> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 My patch to reduce the amount of rtx garbage created: 2014-05-17 Richard Sandiford * emit-rtl.h (replace_equiv_address, replace_equiv_address_nv): Add an inplace argument. Store the new address in the original MEM when true. * emit-rtl.c (change_address_1): Likewise. (adjust_address_1, adjust_automodify_address_1, offset_address): Update accordingly. * rtl.h (plus_constant): Add an inplace argument. * explow.c (plus_constant): Likewise. Try to reuse the original PLUS when true. Avoid generating (plus X (const_int 0)). * function.c (instantiate_virtual_regs_in_rtx): Adjust the PLUS in-place. Pass true to plus_constant. (instantiate_virtual_regs_in_insn): Pass true to replace_equiv_address. exposed a case where a DECL_INCOMING_RTL MEM rtx was being reused in insn patterns without being copied. This meant that instantiating virtual registers changed the DECL_INCOMING_RTL too. The patch fixes this by adding the missing copy_rtxes. However, validize_mem has a bit of an awkward interface as far as sharing goes. If the MEM is already valid, validize_mem returns the original rtx, but if the MEM is not valid it creates a new one. This means that if you copy first you create garbage rtl if the MEM was invalid, whereas if you don't copy first you get invalid sharing if the MEM was valid. Obviously we need to err on the side of copying first, to avoid the invalid sharing. The patch therefore changes the interface so that validize_mem can modify the MEM in-place. I went through all calls to validize_mem to try to find cases where this might cause a problem. The patch fixes up the ones I could see. Most callers already copy first, so as well fixing the bug, this seems to reduce the amount of garbage created. Tested on x86_64-linux-gnu, sparc-sun-solaris2.1? and powerpc64-unknown-linux-gnu. OK to install? Thanks, Richard gcc/ PR middle-end/61268 * function.c (assign_parm_setup_reg): Prevent invalid sharing of DECL_INCOMING_RTL and entry_parm. (get_arg_pointer_save_area): Likewise arg_pointer_save_area. * calls.c (load_register_parameters): Likewise argument values. (emit_library_call_value_1, store_one_arg): Likewise argument save areas. * config/i386/i386.c (assign_386_stack_local): Likewise the local stack slot. * explow.c (validize_mem): Modify the argument in-place. Index: gcc/function.c =================================================================== --- gcc/function.c 2014-07-11 11:55:10.495121493 +0100 +++ gcc/function.c 2014-07-18 08:57:07.047215306 +0100 @@ -2662,13 +2662,14 @@ assign_parm_adjust_entry_rtl (struct ass /* Handle calls that pass values in multiple non-contiguous locations. The Irix 6 ABI has examples of this. */ if (GET_CODE (entry_parm) == PARALLEL) - emit_group_store (validize_mem (stack_parm), entry_parm, + emit_group_store (validize_mem (copy_rtx (stack_parm)), entry_parm, data->passed_type, int_size_in_bytes (data->passed_type)); else { gcc_assert (data->partial % UNITS_PER_WORD == 0); - move_block_from_reg (REGNO (entry_parm), validize_mem (stack_parm), + move_block_from_reg (REGNO (entry_parm), + validize_mem (copy_rtx (stack_parm)), data->partial / UNITS_PER_WORD); } @@ -2837,7 +2838,7 @@ assign_parm_setup_block (struct assign_p else gcc_assert (!size || !(PARM_BOUNDARY % BITS_PER_WORD)); - mem = validize_mem (stack_parm); + mem = validize_mem (copy_rtx (stack_parm)); /* Handle values in multiple non-contiguous locations. */ if (GET_CODE (entry_parm) == PARALLEL) @@ -2972,7 +2973,7 @@ assign_parm_setup_reg (struct assign_par assign_parm_find_data_types and expand_expr_real_1. */ equiv_stack_parm = data->stack_parm; - validated_mem = validize_mem (data->entry_parm); + validated_mem = validize_mem (copy_rtx (data->entry_parm)); need_conversion = (data->nominal_mode != data->passed_mode || promoted_nominal_mode != data->promoted_mode); @@ -3228,7 +3229,7 @@ assign_parm_setup_stack (struct assign_p /* Conversion is required. */ rtx tempreg = gen_reg_rtx (GET_MODE (data->entry_parm)); - emit_move_insn (tempreg, validize_mem (data->entry_parm)); + emit_move_insn (tempreg, validize_mem (copy_rtx (data->entry_parm))); push_to_sequence2 (all->first_conversion_insn, all->last_conversion_insn); to_conversion = true; @@ -3265,8 +3266,8 @@ assign_parm_setup_stack (struct assign_p set_mem_attributes (data->stack_parm, parm, 1); } - dest = validize_mem (data->stack_parm); - src = validize_mem (data->entry_parm); + dest = validize_mem (copy_rtx (data->stack_parm)); + src = validize_mem (copy_rtx (data->entry_parm)); if (MEM_P (src)) { @@ -5261,7 +5262,7 @@ get_arg_pointer_save_area (void) generated stack slot may not be a valid memory address, so we have to check it and fix it if necessary. */ start_sequence (); - emit_move_insn (validize_mem (ret), + emit_move_insn (validize_mem (copy_rtx (ret)), crtl->args.internal_arg_pointer); seq = get_insns (); end_sequence (); Index: gcc/calls.c =================================================================== --- gcc/calls.c 2014-06-22 10:46:24.659598553 +0100 +++ gcc/calls.c 2014-07-18 08:57:07.123215990 +0100 @@ -1937,7 +1937,7 @@ load_register_parameters (struct arg_dat else if (partial == 0 || args[i].pass_on_stack) { - rtx mem = validize_mem (args[i].value); + rtx mem = validize_mem (copy_rtx (args[i].value)); /* Check for overlap with already clobbered argument area, providing that this has non-zero size. */ @@ -4014,7 +4014,8 @@ emit_library_call_value_1 (int retval, r argvec[argnum].locate.size.constant ); - emit_block_move (validize_mem (argvec[argnum].save_area), + emit_block_move (validize_mem + (copy_rtx (argvec[argnum].save_area)), stack_area, GEN_INT (argvec[argnum].locate.size.constant), BLOCK_OP_CALL_PARM); @@ -4289,7 +4290,8 @@ emit_library_call_value_1 (int retval, r if (save_mode == BLKmode) emit_block_move (stack_area, - validize_mem (argvec[count].save_area), + validize_mem + (copy_rtx (argvec[count].save_area)), GEN_INT (argvec[count].locate.size.constant), BLOCK_OP_CALL_PARM); else @@ -4433,7 +4435,8 @@ store_one_arg (struct arg_data *arg, rtx arg->save_area = assign_temp (TREE_TYPE (arg->tree_value), 1, 1); preserve_temp_slots (arg->save_area); - emit_block_move (validize_mem (arg->save_area), stack_area, + emit_block_move (validize_mem (copy_rtx (arg->save_area)), + stack_area, GEN_INT (arg->locate.size.constant), BLOCK_OP_CALL_PARM); } Index: gcc/config/i386/i386.c =================================================================== --- gcc/config/i386/i386.c 2014-07-15 20:49:20.901473328 +0100 +++ gcc/config/i386/i386.c 2014-07-18 08:57:07.110215873 +0100 @@ -25073,7 +25073,7 @@ assign_386_stack_local (enum machine_mod s->next = ix86_stack_locals; ix86_stack_locals = s; - return validize_mem (s->rtl); + return validize_mem (copy_rtx (s->rtl)); } static void Index: gcc/explow.c =================================================================== --- gcc/explow.c 2014-06-22 10:46:24.659598553 +0100 +++ gcc/explow.c 2014-07-18 08:57:07.122215981 +0100 @@ -518,8 +518,9 @@ memory_address_addr_space (enum machine_ return x; } -/* Convert a mem ref into one with a valid memory address. - Pass through anything else unchanged. */ +/* If REF is a MEM with an invalid address, change it into a valid address. + Pass through anything else unchanged. REF must be an unshared rtx and + the function may modify it in-place. */ rtx validize_mem (rtx ref) @@ -531,8 +532,7 @@ validize_mem (rtx ref) MEM_ADDR_SPACE (ref))) return ref; - /* Don't alter REF itself, since that is probably a stack slot. */ - return replace_equiv_address (ref, XEXP (ref, 0)); + return replace_equiv_address (ref, XEXP (ref, 0), true); } /* If X is a memory reference to a member of an object block, try rewriting