Ping. Original message was here:
https://gcc.gnu.org/ml/gcc-patches/2014-07/msg01267.html
Richard Sandiford <rdsandif...@googlemail.com> writes:
> My patch to reduce the amount of rtx garbage created:
>
> 2014-05-17 Richard Sandiford <rdsandif...@googlemail.com>
>
> * 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
