Re: [PATCH] combine: Fix simplify_comparison AND handling for WORD_REGISTER_OPERATIONS targets [PR109040]

[Jakub Jelinek <jakub@redhat.com>]

On Tue, Apr 11, 2023 at 07:26:07PM -0600, Jeff Law wrote:
> I did bootstrap on riscv, but not a regression test, that's spinning right
> now.
> 
> Jeff

> diff --git a/gcc/combine.cc b/gcc/combine.cc
> index 22bf8e1ec89..c41d8a09b3b 100644
> --- a/gcc/combine.cc
> +++ b/gcc/combine.cc
> @@ -10055,9 +10055,10 @@ simplify_and_const_int_1 (scalar_int_mode mode, rtx varop,
>  
>    /* See what bits may be nonzero in VAROP.  Unlike the general case of
>       a call to nonzero_bits, here we don't care about bits outside
> -     MODE.  */
> +     MODE unless WORD_REGISTER_OPERATIONS is true.  */

I would have expected something like
WORD_REGISTER_OPERATIONS && known_le (GET_MODE_PRECISION (mode), BITS_PER_WORD)
as the condition to use word_mode, rather than just
WORD_REGISTER_OPERATIONS.  In both spots.  Because larger modes should be
used as is, not a narrower word_mode instead of them.

> -  nonzero = nonzero_bits (varop, mode) & GET_MODE_MASK (mode);
> +  enum machine_mode tmode = WORD_REGISTER_OPERATIONS ? word_mode : mode;
> +  nonzero = nonzero_bits (varop, tmode) & GET_MODE_MASK (tmode);
>  
>    /* Turn off all bits in the constant that are known to already be zero.
>       Thus, if the AND isn't needed at all, we will have CONSTOP == NONZERO_BITS
> diff --git a/gcc/simplify-rtx.cc b/gcc/simplify-rtx.cc
> index 3b33afa2461..5f6f70491d8 100644
> --- a/gcc/simplify-rtx.cc
> +++ b/gcc/simplify-rtx.cc
> @@ -3752,7 +3752,10 @@ simplify_context::simplify_binary_operation_1 (rtx_code code,
>  	return op0;
>        if (HWI_COMPUTABLE_MODE_P (mode))
>  	{
> -	  HOST_WIDE_INT nzop0 = nonzero_bits (trueop0, mode);
> +	  /* When WORD_REGISTER_OPERATIONS is true, we need to know the
> +	     nonzero bits in WORD_MODE rather than MODE.  */
> +	  HOST_WIDE_INT nzop0
> +	    = nonzero_bits (trueop0, WORD_REGISTER_OPERATIONS ? word_mode : mode);
>  	  HOST_WIDE_INT nzop1;
>  	  if (CONST_INT_P (trueop1))
>  	    {

Regarding my earlier comments for this spot, the later code does
          nzop1 = nonzero_bits (trueop1, mode);
          /* If we are clearing all the nonzero bits, the result is zero.  */
          if ((nzop1 & nzop0) == 0
              && !side_effects_p (op0) && !side_effects_p (op1))
            return CONST0_RTX (mode);
and because nonzero_bits in word_mode if it is wider might have just more
bits set above mode, but nzop1 will not have those bits set, I think it is
fine the way you wrote it (except for the precision check).

	Jakub