On Sun, Dec 10, 2023 at 10:21 AM Andrew Pinski <quic_apin...@quicinc.com> wrote:
>
> After r14-1655-g52c92fb3f40050 (and the other commits
> which touch zero_one_valued_p), we end up with a with
> `bool * a` but where the bool is an SSA name that might not
> have non-zero bits set on it (to 0x1) even though it
> does the non-zero bits would be 0x1.
> The case of coremarks, it is only phiopt4 which adds the new
> ssa name and nothing afterwards updates the nonzero bits on it.
> This fixes the regression by using gimple_zero_one_valued_p
> rather than tree_nonzero_bits to match the cases where the
> SSA_NAME didn't have the non-zero bits set.
> gimple_zero_one_valued_p handles one level of cast and also
> and an `&`.
>
> Bootstrapped and tested on x86_64-linux-gnu.

OK.

I do wonder whether we should do CCP like ops late
to update both alignment and nonzero bits before RTL expansion.
The fold-builtins pass looks spot-on since we remove
assume_aligned & friends there.  I wouldn't exactly fire off CCP
but instead do a RPO walk and just update alignment & nonzero
bits via the bit_value_* helpers and without iteration (using
the SSA name store as lattice).

I'd also say RTL expansion should be re-done creating a
_copy_ of the CFG so we can keep the GIMPLE IL valid
and eventually can use ranger during RTL expansion.

Thanks,
Richard.

> gcc/ChangeLog:
>
>         PR middle-end/112935
>         * expr.cc (expand_expr_real_2): Use
>         gimple_zero_one_valued_p instead of tree_nonzero_bits
>         to find boolean defined expressions.
>
> Signed-off-by: Andrew Pinski <quic_apin...@quicinc.com>
> ---
>  gcc/expr.cc | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/gcc/expr.cc b/gcc/expr.cc
> index 6da51f2aca2..4686cacd22f 100644
> --- a/gcc/expr.cc
> +++ b/gcc/expr.cc
> @@ -10209,8 +10209,9 @@ expand_expr_real_2 (sepops ops, rtx target, 
> machine_mode tmode,
>        /* Expand X*Y as X&-Y when Y must be zero or one.  */
>        if (SCALAR_INT_MODE_P (mode))
>         {
> -         bool bit0_p = tree_nonzero_bits (treeop0) == 1;
> -         bool bit1_p = tree_nonzero_bits (treeop1) == 1;
> +         bool gimple_zero_one_valued_p (tree, tree (*)(tree));
> +         bool bit0_p = gimple_zero_one_valued_p (treeop0, nullptr);
> +         bool bit1_p = gimple_zero_one_valued_p (treeop1, nullptr);
>
>           /* Expand X*Y as X&Y when both X and Y must be zero or one.  */
>           if (bit0_p && bit1_p)
> --
> 2.39.3
>

Reply via email to