https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110308
--- Comment #7 from manolis.tsamis at vrull dot eu ---
Some context for the commit:
This change is originally part of an late rtl pass to optimize memory accesses.
There are a lot of cases (especially involving local arrays) that generate
reduntant stack pointer adds for address calculation which can get reduced to
`mov reg, sp`, but without actually propagating these we don't gain something.
In general it should be good to allow propagation of the stack pointer if that
is correct.
Now for the actual issue, it looks like my change for that was a bit carelees
and I didn't properly understand the context of cprop-hardreg.
> For debug info, propagation of the sp with the extra debug info related stuff
> (ORIGINAL_REGNO and REG_ATTRS) is I think very harmful, but I admit I haven't
> tried to understand why that change has been done.
Yes, the attachment of ORIGINAL_REGNO and REG_ATTRS to the stack pointer has
been accidental and is of course wrong.
I have a proposed change below that fixes the segfault by not setting
ORIGINAL_REGNO/REG_ATTRS for the stack pointer. My understanding is that this
should be fine, but I'm still testing that.
> So I think there are 2 bugs here. First the lost of debugging info because of
> ch, and the latent segfault.
I'm still looking into the difference in the debug statements.
--cut here--
--- a/gcc/regcprop.cc
+++ b/gcc/regcprop.cc
@@ -423,7 +423,7 @@ maybe_mode_change (machine_mode orig_mode, machine_mode
copy_mode,
It's unclear if we need to do the same for other special registers. */
if (regno == STACK_POINTER_REGNUM)
{
- if (orig_mode == new_mode)
+ if (orig_mode == new_mode && new_mode == GET_MODE (stack_pointer_rtx))
return stack_pointer_rtx;
else
return NULL_RTX;
@@ -487,9 +487,14 @@ find_oldest_value_reg (enum reg_class cl, rtx reg, struct
value_data *vd)
new_rtx = maybe_mode_change (oldmode, vd->e[regno].mode, mode, i,
regno);
if (new_rtx)
{
- ORIGINAL_REGNO (new_rtx) = ORIGINAL_REGNO (reg);
- REG_ATTRS (new_rtx) = REG_ATTRS (reg);
- REG_POINTER (new_rtx) = REG_POINTER (reg);
+ if (new_rtx != stack_pointer_rtx)
+ {
+ ORIGINAL_REGNO (new_rtx) = ORIGINAL_REGNO (reg);
+ REG_ATTRS (new_rtx) = REG_ATTRS (reg);
+ REG_POINTER (new_rtx) = REG_POINTER (reg);
+ }
+ else if (REG_POINTER (new_rtx) != REG_POINTER (reg))
+ return NULL_RTX;
return new_rtx;
}
}
@@ -965,15 +970,27 @@ copyprop_hardreg_forward_1 (basic_block bb, struct
value_data *vd)
if (validate_change (insn, &SET_SRC (set), new_rtx, 0))
{
- ORIGINAL_REGNO (new_rtx) = ORIGINAL_REGNO (src);
- REG_ATTRS (new_rtx) = REG_ATTRS (src);
- REG_POINTER (new_rtx) = REG_POINTER (src);
- if (dump_file)
- fprintf (dump_file,
- "insn %u: replaced reg %u with %u\n",
- INSN_UID (insn), regno, REGNO (new_rtx));
- changed = true;
- goto did_replacement;
+ bool can_change;
+ if (new_rtx != stack_pointer_rtx)
+ {
+ ORIGINAL_REGNO (new_rtx) = ORIGINAL_REGNO (src);
+ REG_ATTRS (new_rtx) = REG_ATTRS (src);
+ REG_POINTER (new_rtx) = REG_POINTER (src);
+ can_change = true;
+ }
+ else
+ can_change
+ = (REG_POINTER (new_rtx) == REG_POINTER (src));
+
+ if (can_change)
+ {
+ if (dump_file)
+ fprintf (dump_file,
+ "insn %u: replaced reg %u with %u\n",
+ INSN_UID (insn), regno, REGNO (new_rtx));
+ changed = true;
+ goto did_replacement;
+ }
}
/* We need to re-extract as validate_change clobbers
recog_data. */(In reply to Jakub Jelinek from comment #5)
--cut here--
