The pass 'cprop_hardreg' should not propagate a value from a non-frame-related insn to a frame-related one. This can lead to incorrect dwarf information as noted in PR122274.
cprop_hardreg uses 'struct value_data' to hold lists of registers that contain the same value. However, the value data does not have any information about the instruction that sets the values in the register. In this patch, a new field is added to 'struct value_data_entry' which indicates if the instruction that created this value is frame related or not. Then during copy propagation, we do not replace registers if the copy is happening from a non-frame related insn to a frame related insn. Bootstrapped and regtested on powerpc64le. 2025-11-16 Surya Kumari Jangala <[email protected]> gcc: PR rtl-optimization/122274 * regcprop.cc (value_data_entry): New field. (kill_value_one_regno): Update field. (init_value_data): Initialize field. (kill_set_value_data): New field. (kill_set_value): Update field. (kill_autoinc_value): Likewise. (copy_value): New parameter. Update field. (find_oldest_value_reg): New parameter. Compare frame relatedness of insn and propagated value. (replace_oldest_value_reg): Pass additional parameter to find_oldest_value_reg(). (copyprop_hardreg_forward_1): Pass additional parameter to find_oldest_value_reg(). Compare frame relatedness of insn and propagated value. Call copy_value() with additional parameter. gcc/testsuite: PR rtl-optimization/122274 * gcc.dg/rtl/powerpc/test-frame-related.c: New test. --- gcc/regcprop.cc | 44 ++++++++++++++----- .../gcc.dg/rtl/powerpc/test-frame-related.c | 29 ++++++++++++ 2 files changed, 61 insertions(+), 12 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/rtl/powerpc/test-frame-related.c diff --git a/gcc/regcprop.cc b/gcc/regcprop.cc index 98ab3f77e83..a84fdec0090 100644 --- a/gcc/regcprop.cc +++ b/gcc/regcprop.cc @@ -59,13 +59,16 @@ struct queued_debug_insn_change value. The OLDEST_REGNO field points to the head of the list, and the NEXT_REGNO field runs through the list. The MODE field indicates what mode the data is known to be in; this field is VOIDmode when the - register is not known to contain valid data. */ + register is not known to contain valid data. The FRAME_RELATED field + indicates if the instruction that updated this register is frame + related or not. */ struct value_data_entry { machine_mode mode; unsigned int oldest_regno; unsigned int next_regno; + bool frame_related; struct queued_debug_insn_change *debug_insn_changes; }; @@ -88,12 +91,12 @@ static void set_value_regno (unsigned, machine_mode, struct value_data *); static void init_value_data (struct value_data *); static void kill_clobbered_value (rtx, const_rtx, void *); static void kill_set_value (rtx, const_rtx, void *); -static void copy_value (rtx, rtx, struct value_data *); +static void copy_value (rtx, rtx, struct value_data *, bool frame_related); static bool mode_change_ok (machine_mode, machine_mode, unsigned int); static rtx maybe_mode_change (machine_mode, machine_mode, machine_mode, unsigned int, unsigned int); -static rtx find_oldest_value_reg (enum reg_class, rtx, struct value_data *); +static rtx find_oldest_value_reg (enum reg_class, rtx, struct value_data *, bool frame_related); static bool replace_oldest_value_reg (rtx *, enum reg_class, rtx_insn *, struct value_data *); static bool replace_oldest_value_addr (rtx *, enum reg_class, @@ -147,6 +150,7 @@ kill_value_one_regno (unsigned int regno, struct value_data *vd) vd->e[regno].mode = VOIDmode; vd->e[regno].oldest_regno = regno; vd->e[regno].next_regno = INVALID_REGNUM; + vd->e[regno].frame_related = false; if (vd->e[regno].debug_insn_changes) free_debug_insn_changes (vd, regno); @@ -227,6 +231,7 @@ init_value_data (struct value_data *vd) vd->e[i].oldest_regno = i; vd->e[i].next_regno = INVALID_REGNUM; vd->e[i].debug_insn_changes = NULL; + vd->e[i].frame_related = false; } vd->max_value_regs = 0; vd->n_debug_insn_changes = 0; @@ -248,6 +253,7 @@ struct kill_set_value_data { struct value_data *vd; rtx ignore_set_reg; + bool insn_is_frame_related; }; /* Called through note_stores. If X is set, not clobbered, kill its @@ -264,7 +270,10 @@ kill_set_value (rtx x, const_rtx set, void *data) { kill_value (x, ksvd->vd); if (REG_P (x)) - set_value_regno (REGNO (x), GET_MODE (x), ksvd->vd); + { + set_value_regno (REGNO (x), GET_MODE (x), ksvd->vd); + ksvd->vd->e[REGNO(x)].frame_related = ksvd->insn_is_frame_related; + } } } @@ -283,6 +292,7 @@ kill_autoinc_value (rtx_insn *insn, struct value_data *vd) x = XEXP (x, 0); kill_value (x, vd); set_value_regno (REGNO (x), GET_MODE (x), vd); + vd->e[REGNO(x)].frame_related = RTX_FRAME_RELATED_P (insn); iter.skip_subrtxes (); } } @@ -292,7 +302,7 @@ kill_autoinc_value (rtx_insn *insn, struct value_data *vd) to reflect that SRC contains an older copy of the shared value. */ static void -copy_value (rtx dest, rtx src, struct value_data *vd) +copy_value (rtx dest, rtx src, struct value_data *vd, bool frame_related) { unsigned int dr = REGNO (dest); unsigned int sr = REGNO (src); @@ -391,6 +401,8 @@ copy_value (rtx dest, rtx src, struct value_data *vd) continue; vd->e[i].next_regno = dr; + vd->e[dr].frame_related = frame_related; + if (flag_checking) validate_value_data (vd); } @@ -460,7 +472,7 @@ maybe_mode_change (machine_mode orig_mode, machine_mode copy_mode, of that oldest register, otherwise return NULL. */ static rtx -find_oldest_value_reg (enum reg_class cl, rtx reg, struct value_data *vd) +find_oldest_value_reg (enum reg_class cl, rtx reg, struct value_data *vd, bool frame_related) { unsigned int regno = REGNO (reg); machine_mode mode = GET_MODE (reg); @@ -488,6 +500,9 @@ find_oldest_value_reg (enum reg_class cl, rtx reg, struct value_data *vd) if (!in_hard_reg_set_p (reg_class_contents[cl], mode, i)) continue; + if (frame_related && !vd->e[i].frame_related) + continue; + new_rtx = maybe_mode_change (oldmode, vd->e[regno].mode, mode, i, regno); if (new_rtx) { @@ -513,7 +528,7 @@ static bool replace_oldest_value_reg (rtx *loc, enum reg_class cl, rtx_insn *insn, struct value_data *vd) { - rtx new_rtx = find_oldest_value_reg (cl, *loc, vd); + rtx new_rtx = find_oldest_value_reg (cl, *loc, vd, RTX_FRAME_RELATED_P (insn)); if (new_rtx && (!DEBUG_INSN_P (insn) || !skip_debug_insn_p)) { if (DEBUG_INSN_P (insn)) @@ -807,9 +822,9 @@ copyprop_hardreg_forward_1 (basic_block bb, struct value_data *vd) { unsigned int regno = REGNO (SET_SRC (set)); rtx r1 = find_oldest_value_reg (REGNO_REG_CLASS (regno), - SET_DEST (set), vd); + SET_DEST (set), vd, false); rtx r2 = find_oldest_value_reg (REGNO_REG_CLASS (regno), - SET_SRC (set), vd); + SET_SRC (set), vd, false); if (rtx_equal_p (r1 ? r1 : SET_DEST (set), r2 ? r2 : SET_SRC (set))) { bool last = insn == BB_END (bb); @@ -933,7 +948,7 @@ copyprop_hardreg_forward_1 (basic_block bb, struct value_data *vd) if (REG_P (dest)) { new_rtx = find_oldest_value_reg (REGNO_REG_CLASS (regno), - src, vd); + src, vd, RTX_FRAME_RELATED_P (insn)); if (new_rtx && validate_change (insn, &SET_SRC (set), new_rtx, 0)) { @@ -954,6 +969,8 @@ copyprop_hardreg_forward_1 (basic_block bb, struct value_data *vd) for (i = vd->e[regno].oldest_regno; i != regno; i = vd->e[i].next_regno) { + if (RTX_FRAME_RELATED_P (insn) && !vd->e[i].frame_related) + continue; new_rtx = maybe_mode_change (vd->e[i].mode, vd->e[regno].mode, mode, i, regno); if (new_rtx != NULL_RTX) @@ -1082,6 +1099,7 @@ copyprop_hardreg_forward_1 (basic_block bb, struct value_data *vd) ksvd.vd = vd; ksvd.ignore_set_reg = NULL_RTX; + ksvd.insn_is_frame_related = false; /* Clobber call-clobbered registers. */ if (CALL_P (insn)) @@ -1099,7 +1117,7 @@ copyprop_hardreg_forward_1 (basic_block bb, struct value_data *vd) rtx dest = SET_DEST (x); kill_value (dest, vd); set_value_regno (REGNO (dest), GET_MODE (dest), vd); - copy_value (dest, SET_SRC (x), vd); + copy_value (dest, SET_SRC (x), vd, false); ksvd.ignore_set_reg = dest; set_regno = REGNO (dest); set_nregs = REG_NREGS (dest); @@ -1147,6 +1165,8 @@ copyprop_hardreg_forward_1 (basic_block bb, struct value_data *vd) if (!noop_p) { + ksvd.insn_is_frame_related = RTX_FRAME_RELATED_P (insn); + /* Notice stores. */ note_stores (insn, kill_set_value, &ksvd); @@ -1154,7 +1174,7 @@ copyprop_hardreg_forward_1 (basic_block bb, struct value_data *vd) if (copy_p) { df_insn_rescan (insn); - copy_value (SET_DEST (set), SET_SRC (set), vd); + copy_value (SET_DEST (set), SET_SRC (set), vd, RTX_FRAME_RELATED_P (insn)); } } diff --git a/gcc/testsuite/gcc.dg/rtl/powerpc/test-frame-related.c b/gcc/testsuite/gcc.dg/rtl/powerpc/test-frame-related.c new file mode 100644 index 00000000000..95599d02bae --- /dev/null +++ b/gcc/testsuite/gcc.dg/rtl/powerpc/test-frame-related.c @@ -0,0 +1,29 @@ +/* { dg-do compile { target { powerpc64*-*-linux* } } } */ +/* { dg-options "-O2 -mregnames" } */ + +int __RTL (startwith ("cprop_hardreg")) test_frame_related () +{ +(function "frame_related" + (insn-chain + (block 2 + (edge-from entry (flags "FALLTHRU")) + (cnote 3 [bb 2] NOTE_INSN_BASIC_BLOCK) + (cinsn 8 (set (reg:DI %r5) + (reg:DI lr))) + (cinsn/f 9 (set (reg:DI %r0) + (reg:DI lr))) + (cinsn/f 10 (set (mem/c:DI (plus:DI (reg/f:DI %r1) + (const_int 16 ))[18 S8 A8]) + (reg:DI %r0))) + + ;; Extra insn to avoid the above being deleted by DCE. + (cinsn 18 (use (reg:DI %r5))) + (edge-to exit (flags "FALLTHRU")) + ) ;; block 2 + ) ;; insn-chain +) ;; function "main" +} + +/* { dg-final { scan-assembler {\mmflr %r0\M} } } */ +/* { dg-final { scan-assembler {\mmflr %r5\M} } } */ +/* { dg-final { scan-assembler {\mstd %r0,16\(%r1\)} } } */ -- 2.47.3
