Re: [PATCH v7] forwprop: add simplify_phi_result_movdiv() [PR101179]

On Thu, Aug 13, 2026 at 8:06 AM Jeffrey Law
<[email protected]> wrote:
>
> On 8/12/2026 2:42 PM, Daniel Barboza wrote:
> > This new forwprop  step is my attempt to implement Richi's suggestions
> > from v1 of this work [1] where he suggested to push things out of
> > match.pd.
> >
> > The idea is to simplify DIV/MOD into RSHIFT/BIT_AND ops in which the
> > divisor are pow2 integers in a PHI.  E.g.:
> >
> > - for MOD, if either "_y" is known positive or "_x" is used just in zero
> >    comparisons:
> >
> > phi_var = PHI <16,4>
> > _x = _y % phi_var
> >
> > Can be turned into:
> >
> > phi_var = PHI <15,3>
> > _x = _y & phi_var
> >
> > - for DIV, if "_y" is a known positive:
> >
> > phi_var = PHI <16,4>
> > _x = _y / phi_var
> >
> > Can be turned into:
> >
> > phi_var = PHI <4,2>
> > _x = _y >> phi_var
> >
> > Most of 101179 use cases are solved by this change.  PHI with 2+ args
> > are supported as long as every phi_arg meets the criteria.
> >
> > Boostrapped and regression tested with x86_64, aarch64 and riscv64.
> >
> > [1] https://gcc.gnu.org/pipermail/gcc-patches/2026-May/716303.html
> >
> >       PR tree-optimization/101179
> >
> > gcc/ChangeLog:
> >
> >       * tree-ssa-forwprop.cc (simplify_phi_result_movdiv): New
> >       forwprop step where MOD/DIV ops with pow2 divisors can be
> >       simplified to BIT_AND/RSHIFT.
> >       (pass_forwprop::execute): Call simplify_phi_result_movdiv.
> >
> > gcc/testsuite/ChangeLog:
> >
> >       * gcc.dg/tree-ssa/pr101179.c: New test.
> So I hate to have to bring up more issues, but I think this is broken
> for CEIL and ROUND variants.
>
> Let's consider  3/4 with CEIL_DIV_EXPR, the result of that should be 1.
> But I think with this patch we get 0.  Similarly for ROUND_DIV_EXPR and
> the MOD variants I believe.
>
>
> > +
> > +  tree type = TREE_TYPE (gimple_phi_result (phi));
> > +  tree new_phires = make_ssa_name (type);
> > +  gphi *new_phi = create_phi_node (new_phires, phi->bb);
> > +
> > +  for (unsigned int i = 0; i < gimple_phi_num_args (phi); i++)
> > +    {
> > +      tree phi_arg = gimple_phi_arg_def (phi, i);
> > +      tree arg;
> > +
> > +      if (new_code == RSHIFT_EXPR)
> > +     arg = wide_int_to_tree (type, wi::exact_log2 (wi::to_wide (phi_arg)));
> > +      else
> > +     arg = wide_int_to_tree (type, wi::to_wide (phi_arg) - 1);
> > +
> > +      SET_PHI_ARG_DEF (new_phi, i, arg);
> > +    }
> Do you want to also copy any location information from the old phi to
> new phi?  See gimple_phi_arg_location and friends for the API to get and
> set the locations.  You might also want to look at add_phi_arg, it may
> (or may not) be a better API to work with.
>
>
>
> > +
> > +  /* Add a gimple_convert to integer_type_node for new_phires
> > +     since it might be a long long which we want to convert
> > +     into an integer or a bit_int that we want to convert into
> > +     an integer.  */
> > +  gimple_stmt_iterator gsi;
> > +  if (new_code == RSHIFT_EXPR)
> > +    {
> > +      gsi = gsi_for_stmt (stmt);
> > +      new_phires = gimple_convert (&gsi, true, GSI_SAME_STMT,
> > +                                gimple_location (stmt),
> > +                                integer_type_node, new_phires);
> > +    }
> Is integer_type_node the right type here?  Shouldn't we be getting the
> type from somewhere in the IL rather than assuming integer_type_node is
> correct?

For shift's rhs integer_type_node is correct; the gimplifier (or maybe
the C front-end I forget which one) converts shfit's rhs to int
always.

>
> jeff