Sorry for the long delay. SP_DERIVED_VALUE_P still deals with stack-pointer related situations, and since there have been optimizations and fixes targeting stack access specifically, it does not seem correct to simply replace it. (Actually there were regressions when I tried.)
As for the debug info issue, the issue in the original commit seems to be related to frame pointers, which does not seem a problem for other registers. I retained the original behaviors of SP_DERIVED_VALUE_P, so that should be fine. I am not that certain, however, and maybe Jakub could have a look at it... Bohan ------------------------------------------------------------------ From:Jeff Law <[email protected]> Send Time:2025 Jun. 10 (Tue.) 07:59 To:Bohan Lei<[email protected]>; "gcc-patches"<[email protected]> CC:jakub<[email protected]> Subject:Re: [RFC PATCH v2] cselib: Reuse VALUEs on reg adjustments On 12/3/24 9:57 PM, Bohan Lei wrote: > This is v2 of the patch in > https://gcc.gnu.org/pipermail/gcc-patches/2024-November/669380.html. > I missed the ChangeLog entry in that version. > > The commit 2c0fa3ecf70d199af18785702e9e0548fd3ab793 reuses VALUEs on sp > adjustments. We can generalize the idea and reuse VALUEs on other > registers. This can help the postreload pass find more opportunities to > simplify insns. > > The following assembly code is generated from the testcase using the current > trunk compiler: > > .L5: > movq %rbp, %rsi > movq %rbx, %rdi > call l > movq %rbx, %rsi > addq $4, %rbx > testl %eax, %eax > je .L6 > leaq -4(%rbx), %rax > cmpq %rax, %rbp > je .L6 > movq %rbx, %rdi > call as > > The lea instruction is actually redundant here, as rsi contains the same > value as rbx-4 and can be used in the cmp instruction instead of rax. > With this patch, the lea instruction can be eliminated in the postreload > pass. > > Bootstrapped and regtested on x86_64-pc-linux-gnu, no regressions. > > gcc/ChangeLog: > > * cselib.cc (REG_DERIVED_VALUE_P): New macro. > (cselib_hasher::equal): Use REG_DERIVED_VALUE_P in place of > SP_DERIVED_VALUE_P. > (autoinc_split): Ditto. > (rtx_equal_for_cselib_1): Ditto. > (cselib_hash_plus_const_int): Ditto. > (cselib_subst_to_values): Ditto. > (cselib_lookup_1): Set REG_DERIVED_VALUE_P for newly created > VALUEs for registers. Use REG_DERIVED_VALUE_P in place of > SP_DERIVED_VALUE_P for PRESERVED_VALUE_P logic. > * rtl.h (struct GTY): Mention that the volatil bit is used as > REG_DERIVED_VALUE_P in cselib.cc. > > gcc/testsuite/ChangeLog: > > * gcc.target/i386/cselib-1.c: New test. > --- > gcc/cselib.cc | 29 ++++++++++++++---------- > gcc/rtl.h | 1 + > gcc/testsuite/gcc.target/i386/cselib-1.c | 22 ++++++++++++++++++ > 3 files changed, 40 insertions(+), 12 deletions(-) > create mode 100644 gcc/testsuite/gcc.target/i386/cselib-1.c > > diff --git a/gcc/cselib.cc b/gcc/cselib.cc > index e6a36e892bb..43ab266c4de 100644 > --- a/gcc/cselib.cc > +++ b/gcc/cselib.cc > @@ -70,6 +70,9 @@ static rtx autoinc_split (rtx, rtx *, machine_mode); > #define SP_DERIVED_VALUE_P(RTX) \ > (RTL_FLAG_CHECK1 ("SP_DERIVED_VALUE_P", (RTX), VALUE)->call) > > +#define REG_DERIVED_VALUE_P(RTX) \ > + (RTL_FLAG_CHECK1 ("REG_DERIVED_VALUE_P", (RTX), VALUE)->volatil) > + Are there any cases where SP_DERIVED_VALUE_P is still used after your proposed change? If not, then we probably should drop SP_DERIVED_VALUE_P. Does this potentially impact debug info? That was one of the big issues Jakub spent time on with the original patch. That's my only conceptual concern. Implementation wise it seems pretty straightforward. Jeff
