On Thu, Jul 24, 2014 at 8:58 AM, Richard Sandiford
<rdsandif...@googlemail.com> wrote:
> Ping. Original message was here:
>
> https://gcc.gnu.org/ml/gcc-patches/2014-07/msg01267.html
Ok.
Thanks,
Richard.
> 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
