Hi! Thank you for doing this Surya!
On Mon, Nov 17, 2025 at 12:52:57PM +0530, Surya Kumari Jangala wrote: > 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 Two spaces after a full stop. > + indicates if the instruction that updated this register is frame > + related or not. */ And again. Maybe it is easier to just store the rtx_insn here? > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/rtl/powerpc/test-frame-related.c > @@ -0,0 +1,29 @@ > +/* { dg-do compile { target { powerpc64*-*-linux* } } } */ What do you actually want to test for? Certainly not if the target triple starts with that string! There should probably be a comment here, too (unless you can make it *very* obvious). > +/* { dg-final { scan-assembler {\mmflr %r0\M} } } */ > +/* { dg-final { scan-assembler {\mmflr %r5\M} } } */ > +/* { dg-final { scan-assembler {\mstd %r0,16\(%r1\)} } } */ That register syntax is not the default used on any of our targets. You made the test to be run only on Linux targets, but otherwise this can even never work! (The usual syntax is just "mflr 5" etc.) Segher
