Soumya AR <[email protected]> writes:
> The current narrow_gp_writes pass makes a single sweep over a function in RPO,
> and makes conservative assumptions about definitions that have not yet been
> processed (eg. from backedges).
>
> Following the suggestions on the original pass at
> gcc.gnu.org/pipermail/gcc-patches/2026-February/709227.html, this pass has 
> been
> rewritten to use an iterative worklist analysis, which uses an optimistic
> approach to find more narrowing opportunities.
>
> With this update, we observe around 1600 more narrowings (+3.35%) with 
> SPEC2026
> INT benchmarks.
>
> ----
>
> Consider the following reduced example from 706.stockfish_r:
>
>   unsigned long long sum = 0;
>   x &= 255;
>
>   while (x)
>     {
>       sum += x;
>       x &= x - 1;
>     }
>
> At the loop header, a PHI merges the initial value of x with its value
> from the backedge. The phi's nonzero-bit mask is the bitwise OR of the masks
> of all its incoming definitions.
>
> The initial value has mask 0xff, but the old pass assumes a full-width
> mask for the unprocessed backedge, making the PHI mask full-width too.
>
> With a zero starting mask for the backedge, the PHI mask is
> 0xff | 0 = 0xff. The loop's AND preserves this bound, giving the backedge
> a mask of 0xff. Revisiting the PHI gives 0xff | 0xff = 0xff, so the masks
> are stable and the AND can be narrowed to 32 bits.
>
> ----
>
> We separate the worklist analysis into a generic RTL-SSA helper that is used
> by the aarch64 narrowing pass. This generic framework works using two 
> callbacks
> supplied by the client (caller):
>    - can_process_def_p (def) returns true if the client wants this def
>      processed (ie. added to the worklist)
>    - process_def (def) performs a client-specific computation on def and
>      returns true if definitions that depend on def need to be processed
>      again.
>
> Each selected definition starts with an optimistic zero mask, stored in
> m_nonzero_masks.
>
> These definitions are added to a worklist and processed one at a time.
> Instruction masks are computed using nonzero_bits, while PHI masks are 
> computed
> as the bitwise OR of their input masks. Whenever a mask changes, its entry in
> the map is updated, and selected definitions that depend on it are enqueued 
> for
> further processing.
>
> We continue until the worklist is empty. At this point, processing the
> definitions again would not change any mask since the analysis has reached a
> fixed point. We then use these masks to narrow instructions.
>
> ----
>
> Bootstrapped and regtested on aarch64-linux-gnu, no regressions. OK for trunk?
>
> gcc/ChangeLog:
>
>       * Makefile.in: Add rtl-ssa/def-propagation.h.
>       * config/aarch64/aarch64-narrow-gp-writes.cc: Update to use worklist
>       algorithm.
>       * rtl-ssa.h: Include def-propagation.h
>       * rtl-ssa/def-propagation.h: New file.
>
> gcc/testsuite/ChangeLog:
>
>       * gcc.target/aarch64/narrow-gp-writes-9.c: New test.
>       
> Signed-off-by: Soumya AR <[email protected]>
>
> From 580249f4a6668d4eaf8e173d2629d7e629425f86 Mon Sep 17 00:00:00 2001
> From: Soumya AR <[email protected]>
> Date: Tue, 8 Sep 2026 09:43:38 +0000
> Subject: [PATCH] aarch64: Use optimistic worklist analysis for
>  narrow_gp_writes
>
> The current narrow_gp_writes pass makes a single sweep over a function in RPO,
> and makes conservative assumptions about definitions that have not yet been
> processed (eg. from backedges).
>
> Following the suggestions on the original pass at
> gcc.gnu.org/pipermail/gcc-patches/2026-February/709227.html, this pass has 
> been
> rewritten to use an iterative worklist analysis, which uses an optimistic
> approach to find more narrowing opportunities.
>
> With this update, we observe around 1600 more narrowings (+3.35%) with 
> SPEC2026
> INT benchmarks.
>
> ----
>
> Consider the following reduced example from 706.stockfish_r:
>
>   unsigned long long sum = 0;
>   x &= 255;
>
>   while (x)
>     {
>       sum += x;
>       x &= x - 1;
>     }
>
> At the loop header, a PHI merges the initial value of x with its value
> from the backedge. The phi's nonzero-bit mask is the bitwise OR of the masks
> of all its incoming definitions.
>
> The initial value has mask 0xff, but the old pass assumes a full-width
> mask for the unprocessed backedge, making the PHI mask full-width too.
>
> With a zero starting mask for the backedge, the PHI mask is
> 0xff | 0 = 0xff. The loop's AND preserves this bound, giving the backedge
> a mask of 0xff. Revisiting the PHI gives 0xff | 0xff = 0xff, so the masks
> are stable and the AND can be narrowed to 32 bits.
>
> ----
>
> We separate the worklist analysis into a generic RTL-SSA helper that is used
> by the aarch64 narrowing pass. This generic framework works using two 
> callbacks
> supplied by the client (caller):
>    - can_process_def_p (def) returns true if the client wants this def
>      processed (ie. added to the worklist)
>    - process_def (def) performs a client-specific computation on def and
>      returns true if definitions that depend on def need to be processed
>      again.
>
> Each selected definition starts with an optimistic zero mask, stored in
> m_nonzero_masks.
>
> These definitions are added to a worklist and processed one at a time.
> Instruction masks are computed using nonzero_bits, while PHI masks are 
> computed
> as the bitwise OR of their input masks. Whenever a mask changes, its entry in
> the map is updated, and selected definitions that depend on it are enqueued 
> for
> further processing.
>
> We continue until the worklist is empty. At this point, processing the
> definitions again would not change any mask since the analysis has reached a
> fixed point. We then use these masks to narrow instructions.
>
> ----
>
> Bootstrapped and regtested on aarch64-linux-gnu, no regressions. OK for trunk?
>
> gcc/ChangeLog:
>
>       * Makefile.in: Add rtl-ssa/def-propagation.h.
>       * config/aarch64/aarch64-narrow-gp-writes.cc: Update to use worklist
>       algorithm.
>       * rtl-ssa.h: Include def-propagation.h
>       * rtl-ssa/def-propagation.h: New file.
>
> gcc/testsuite/ChangeLog:
>
>       * gcc.target/aarch64/narrow-gp-writes-9.c: New test.
>
> Signed-off-by: Soumya AR <[email protected]>

Thanks for doing this.  It mostly looks good to me, but a couple of
comments below:

> [...]
> +/* RTL hooks callback for computing nonzero bits of registers.
> +   Updates NONZERO with the tracked nonzero bits for register X.  */
> +rtx
> +reg_nonzero_bits_for_narrow_gp_writes (const_rtx x, scalar_int_mode xmode,
> +                                    scalar_int_mode, mask_t *nonzero)
> +{
> +  narrow_gp_writes *pass = narrow_gp_writes::get_instance ();
> +  insn_info *insn = pass->m_current_insn;
> +  if (!insn)
> +    return NULL_RTX;
> +
> +  use_info *use = find_access (insn->uses (), REGNO (x));
> +  if (!use)
> +    return NULL_RTX;
> +  /* find_access searches by register number, not mode.  Ignore the result if
> +     its mode does not match XMODE.  */
> +  if (use->mode () != xmode)
> +    return NULL_RTX;
> +  set_info *reaching_def = use->def ();
> +  if (!reaching_def)
> +    return NULL_RTX;
> +
> +  mask_t def_mask = pass->lookup_nonzero_mask (reaching_def);
> +  /* If the def is in a narrower mode than the use, RTL semantics say the 
> upper
> +     bits are undefined.  Treat them conservatively as potentially nonzero.  
> */
> +  if (known_lt (GET_MODE_SIZE (reaching_def->mode ()),
> +             GET_MODE_SIZE (use->mode ())))

This is pre-existing, but it should use maybe_lt rather than known_lt.

> +    def_mask |= ~GET_MODE_MASK (reaching_def->mode ());
> +  *nonzero &= def_mask;
> +  return NULL_RTX;
> +}
> +
> +/* Return true if DEF is a GP register definition that can be analyzed.  */
> +bool
> +narrow_gp_writes::can_track_def_p (set_info *def) const
> +{
> +  if (!GP_REGNUM_P (def->regno ()))
> +    return false;
>  
> -   The caller is expected to call this once per phi and store the result;
> -   calling it on demand would re-walk all phi inputs for every use, giving
> -   O(in-degree * num-uses) cost.  */
> +  insn_info *insn = def->insn ();
> +  if (insn->is_phi ())
> +    return true;
>  
> +  return can_track_nonzero_bits_p (insn);
> +}
> +
> +/* Collect the definitions to analyze and initialize each definition's 
> nonzero
> +   mask to zero.  This is an optimistic assumption that will be refined as we
> +   process more information.  */
> +void
> +narrow_gp_writes::collect_initial_defs (auto_vec<set_info *> &initial_defs)
> +{
> +  for (ebb_info *ebb : crtl->ssa->ebbs ())
> +    {
> +      for (phi_info *phi : ebb->phis ())
> +     if (can_track_def_p (phi))
> +       {
> +         m_nonzero_masks.put (phi, 0);
> +         initial_defs.safe_push (phi);
> +       }
> +
> +      for (insn_info *insn : ebb->nondebug_insns ())
> +     for (def_info *def : insn->defs ())
> +       if (set_info *set = dyn_cast<set_info *> (def))
> +         if (can_track_def_p (set))
> +           {
> +             m_nonzero_masks.put (set, 0);
> +             initial_defs.safe_push (set);
> +           }
> +    }

This seems broader than it needs to be.  We should only need to make
a conservative assumption for backedges.

How about handling this as follows:

(1) Remove the initial_defs argument from def_propagation::propagate.

(2) Instead, make the first pass of def_propagation::propagate
    iterate over all definitions itself.  Like the existing loop,
    this would involve calling process_def, but it would only enqueue
    uses that occur earlier than the current instruction.  (Later uses
    will be processed later in the first pass.)

(3) Expose can_track_nonzero_bits_p to def_propagation with an
    appropriate abstraction (e.g. can_process_insn_p, but better
    names welcome).  Use it to skip processing of instructions in (2).

(4) Add a new parameter to process_def that indicates whether it is
    being called during the first pass.

(5) Treat backedges optimistically on the first pass.  This would only
    affect calculate_phi_mask.

(6) Similarly, make process_def assume that the definition's current
    mask is 0 rather than GET_MODE_MASK during the first pass.
    Perhaps a helper would be good, given that (5) and (6) need
    the same thing.

(7) Make process_def remove items from m_nonzero_masks rather than
    storing a superset of GET_MODE_MASK.

This should be more efficient in two ways: it saves one pass over the
IL and it avoids creating m_nonzero_masks entries for definitions that
never have useful information.

Thanks,
Richard

Reply via email to