On Tue, 24 Feb 2026 at 22:09, Andrew Pinski
<[email protected]> wrote:
>
> On Tue, Feb 24, 2026 at 1:49 AM Philipp Tomsich
> <[email protected]> wrote:
> >
> > Fix three independent issues in the avoid-store-forwarding pass:
>
>
> Can you split these up before committing them?

Sure. Those were only squashed for the patch submission anyway.
I'll commit separately and repost as 'COMMITTED'.

Thank you for taking the time to review right away,
Philipp.

>
> >
> > 1. The "Cases of store forwarding avoided" statistic was incorrectly
> >    reporting stats_sf_detected instead of stats_sf_avoided.
>
> Ok.
>
> >
> > 2. The sbitmap allocated in process_store_forwarding was never freed.
> >    Replace sbitmap_alloc with auto_sbitmap so the destructor handles
> >    cleanup on all return paths.
>
> You don't need bitmap_clear (forwarded_bytes); after declaring the
> auto_bitmap so remove that too; otherwise ok.
>
> >
> > 3. The header file included config.h, system.h, coretypes.h,
> >    backend.h, and rtl.h directly, which is unconventional for GCC
> >    headers.  Strip those includes from the header and fix the include
> >    order in the .cc file so that config.h comes first.  Also remove
> >    the unused cselib.h include.
>
> Ok
>
> Thanks,
> Andre
>
> >
> > gcc/ChangeLog:
> >
> >         * avoid-store-forwarding.cc: Move config.h before
> >         avoid-store-forwarding.h; place avoid-store-forwarding.h
> >         after rtl.h; remove unused cselib.h include.
> >         (store_forwarding_analyzer::process_store_forwarding): Use
> >         auto_sbitmap instead of sbitmap_alloc.
> >         (store_forwarding_analyzer::update_stats): Fix second
> >         statistics_counter_event to use stats_sf_avoided.
> >         * avoid-store-forwarding.h: Remove includes of config.h,
> >         system.h, coretypes.h, backend.h, rtl.h.
> >
> > Reviewed-By: Konstantinos Eleftheriou <[email protected]>
> > ---
> >  gcc/avoid-store-forwarding.cc | 7 +++----
> >  gcc/avoid-store-forwarding.h  | 6 ------
> >  2 files changed, 3 insertions(+), 10 deletions(-)
> >
> > diff --git a/gcc/avoid-store-forwarding.cc b/gcc/avoid-store-forwarding.cc
> > index 86292ce574a2..75ba30a62eb5 100644
> > --- a/gcc/avoid-store-forwarding.cc
> > +++ b/gcc/avoid-store-forwarding.cc
> > @@ -18,18 +18,17 @@
> >     along with GCC; see the file COPYING3.  If not see
> >     <http://www.gnu.org/licenses/>.  */
> >
> > -#include "avoid-store-forwarding.h"
> >  #include "config.h"
> >  #include "system.h"
> >  #include "coretypes.h"
> >  #include "backend.h"
> >  #include "target.h"
> >  #include "rtl.h"
> > +#include "avoid-store-forwarding.h"
> >  #include "alias.h"
> >  #include "rtlanal.h"
> >  #include "cfgrtl.h"
> >  #include "tree-pass.h"
> > -#include "cselib.h"
> >  #include "predict.h"
> >  #include "insn-config.h"
> >  #include "expmed.h"
> > @@ -177,7 +176,7 @@ process_store_forwarding (vec<store_fwd_info> &stores, 
> > rtx_insn *load_insn,
> >       We can also eliminate stores on addresses that are overwritten
> >       by later stores.  */
> >
> > -  sbitmap forwarded_bytes = sbitmap_alloc (load_size);
> > +  auto_sbitmap forwarded_bytes (load_size);
> >    bitmap_clear (forwarded_bytes);
> >
> >    unsigned int i;
> > @@ -715,7 +714,7 @@ store_forwarding_analyzer::update_stats (function *fn)
> >    statistics_counter_event (fn, "Cases of store forwarding detected: ",
> >                             stats_sf_detected);
> >    statistics_counter_event (fn, "Cases of store forwarding avoided: ",
> > -                           stats_sf_detected);
> > +                           stats_sf_avoided);
> >  }
> >
> >  unsigned int
> > diff --git a/gcc/avoid-store-forwarding.h b/gcc/avoid-store-forwarding.h
> > index 1650d687d544..8f455c6af7dc 100644
> > --- a/gcc/avoid-store-forwarding.h
> > +++ b/gcc/avoid-store-forwarding.h
> > @@ -21,12 +21,6 @@
> >  #ifndef GCC_AVOID_STORE_FORWARDING_H
> >  #define GCC_AVOID_STORE_FORWARDING_H
> >
> > -#include "config.h"
> > -#include "system.h"
> > -#include "coretypes.h"
> > -#include "backend.h"
> > -#include "rtl.h"
> > -
> >  struct store_fwd_info
> >  {
> >    /* The store instruction that is a store forwarding candidate.  */
> > --
> > 2.34.1
> >

Reply via email to