PR 61628: Invalid sharing of DECL_INCOMING_RTL
Richard Sandiford
rdsandiford@googlemail.com
Fri Jul 18 09:05:00 GMT 2014
My patch to reduce the amount of rtx garbage created:
2014-05-17 Richard Sandiford <rdsandiford@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
More information about the Gcc-patches
mailing list