On Fri, May 19, 2023 at 4:15 AM Andrew Pinski via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> While working something else, I noticed we could improve
> the following function code generation:
> ```
> unsigned f(unsigned t)
> {
> if (t & ~(1<<30)) __builtin_unreachable();
> return t != 0;
> }
> ```
> Right know we just emit a comparison against 0 instead
> of just a shift right by 30.
> There is code in do_store_flag which already optimizes
> `(t & 1<<30) != 0` to `(t >> 30) & 1`. This patch
> extends it to handle the case where we know t has a
> nonzero of just one bit set.
>
> OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.
>
> gcc/ChangeLog:
>
> * expr.cc (do_store_flag): Extend the one bit checking case
> to handle the case where we don't have an and but rather still
> one bit is known to be non-zero.
> ---
> gcc/expr.cc | 27 +++++++++++++++++++++------
> 1 file changed, 21 insertions(+), 6 deletions(-)
>
> diff --git a/gcc/expr.cc b/gcc/expr.cc
> index 5ede094e705..91528e734e7 100644
> --- a/gcc/expr.cc
> +++ b/gcc/expr.cc
> @@ -13083,15 +13083,30 @@ do_store_flag (sepops ops, rtx target, machine_mode
> mode)
> && integer_zerop (arg1)
> && (TYPE_PRECISION (ops->type) != 1 || TYPE_UNSIGNED (ops->type)))
> {
> - gimple *srcstmt = get_def_for_expr (arg0, BIT_AND_EXPR);
> - if (srcstmt
> - && integer_pow2p (gimple_assign_rhs2 (srcstmt)))
> + wide_int nz = tree_nonzero_bits (arg0);
> +
> + if (wi::popcount (nz) == 1)
> {
> + tree op0;
> + tree op1;
> + gimple *srcstmt = get_def_for_expr (arg0, BIT_AND_EXPR);
> + /* If the defining statement was (x & POW2), then remove the and
> + as we are going to add it back. */
> + if (srcstmt
> + && integer_pow2p (gimple_assign_rhs2 (srcstmt)))
> + {
> + op0 = gimple_assign_rhs1 (srcstmt);
> + op1 = gimple_assign_rhs2 (srcstmt);
> + }
> + else
> + {
> + op0 = arg0;
> + op1 = wide_int_to_tree (TREE_TYPE (op0), nz);
> + }
> enum tree_code tcode = code == NE ? NE_EXPR : EQ_EXPR;
> type = lang_hooks.types.type_for_mode (mode, unsignedp);
> - tree temp = fold_build2_loc (loc, BIT_AND_EXPR, TREE_TYPE (arg1),
> - gimple_assign_rhs1 (srcstmt),
> - gimple_assign_rhs2 (srcstmt));
> + tree temp = fold_build2_loc (loc, BIT_AND_EXPR, TREE_TYPE (op0),
> + op0, op1);
> temp = fold_single_bit_test (loc, tcode, temp, arg1, type);
> if (temp)
> return expand_expr (temp, target, VOIDmode, EXPAND_NORMAL);
I wonder if, instead of expanding expand with these kind of tricks we
want to instead
add to ISEL and use direct optab IFNs for things we matched? In
particular I think
we do want to get rid of TER but the above adds another use of get_def_for_expr.
As Jeff says the above doesn't look like it includes costing so that would be an
argument to make it a generic match.pd transform (it appears to be "simpler")?
Richard.
> --
> 2.31.1
>
