Re: [PATCH] powerpc: Fix up *branch_anddi3_dot for -m32 -mpowerpc64 [PR109566]

[Segher Boessenkool <segher@kernel.crashing.org>]

Hi!

On Mon, Apr 24, 2023 at 05:54:02PM +0200, Jakub Jelinek wrote:
> The problem is that the *branch_anddi3_dot define_insn_and_split
> relies on the *rotldi3_mask_dot define_insn_and_split being recognized
> during splitting.  The rs6000_is_valid_rotate_dot_mask function checks whether
> the mask is a CONST_INT which is a valid mask, but *rotl<mode>3_mask_dot in
> addition to checking that it is a valid mask also has
>   (<MODE>mode == Pmode || UINTVAL (operands[3]) <= 0x7fffffff)
> test in the condition.  For TARGET_64BIT that doesn't add any further
> requirements, but for !TARGET_64BIT && TARGET_POWERPC64 if the AND
> second operand is larger than INT_MAX it will not be recognized.

The reason is that code that runs in 32-bit mode, (MSR[SF]=0, which is
what you get with -m32 code) does the comparisons for record-form insns
("dot insns") as 32-bit compares.  Often this matters.  But if your insn
masks with (some subset of) 0x7fffffff all three result bits (LT, GT,
EQ, meaning the sign bit is set, the sign bit is not set but some other
value bit is, and nobits are set, respectively) are set the same in
either mode.

> --- gcc/config/rs6000/rs6000.cc.jj	2023-04-04 10:33:47.433201866 +0200
> +++ gcc/config/rs6000/rs6000.cc	2023-04-24 12:31:07.237031550 +0200
> @@ -11409,7 +11409,16 @@ bool
>  rs6000_is_valid_rotate_dot_mask (rtx mask, machine_mode mode)
>  {
>    int nb, ne;
> -  return rs6000_is_valid_mask (mask, &nb, &ne, mode) && nb >= ne && ne > 0;
> +  if (rs6000_is_valid_mask (mask, &nb, &ne, mode) && nb >= ne && ne > 0)
> +    {
> +      if (TARGET_64BIT)
> +	return true;
> +      /* *rotldi3_mask_dot requires for -m32 -mpowerpc64 that the mask is
> +	 <= 0x7ffffff.  */
> +      return (UINTVAL (mask) << (63 - nb)) <= 0x7fffffff;
> +    }
> +  else
> +    return false;

No superfluous "else" please, just put a blank line there.

Okay for trunk (with Ke Wen's remarks fixed, but you already did :-) )
Thanks!


Segher