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

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?

jeff