Re: [PATCH][i386]: Update ix86_can_change_mode_class target hook to accept QImode conversions

[Hongtao Liu <crazylht@gmail.com>]

On Fri, Nov 11, 2022 at 10:47 PM Tamar Christina via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Hi All,
>
> The current i386 implementation of the TARGET_CAN_CHANGE_MODE_CLASS is currently
> not useful before re-alloc.
>
> In particular before regalloc optimization passes query the hook using ALL_REGS,
> but because of the
>
>   if (MAYBE_FLOAT_CLASS_P (regclass))
>       return false;
>
> The hook returns false for all modes, even integer ones because ALL_REGS
> overlaps with floating point regs.
>
> The vector permute fallback cases used to unconditionally convert vector integer
> permutes to vector QImode ones as a fallback plan.  This is incorrect and can
> result in incorrect code if the target doesn't support this conversion.
>
> To fix this some more checks were added, however that ended up introducing ICEs
> in the i386 backend because e.g. the hook would reject conversions between modes
> like V2TImode and V32QImode.
>
> My understanding is that for x87 we don't want to allow floating point
> conversions, but integers are fine.  So I have modified the check such that it
> also checks the modes, not just the register class groups.
>
> The second part of the code is needed because now that integer modes aren't
> uniformly rejected the i386 backend trigger further optimizations.  However the
> backend lacks instructions to deal with canonical RTL representations of
> certain instructions.  for instance the back-end seems to prefer vec_select 0
> instead of subregs.
If the canonical form of the lower half of the vector in rtl is subreg
instead of vec_select, the x86 backend needs to update the pattern
accordingly to avoid diverge.
I can reproduce those failures when adding VALID_FP_MODE_P in
targetm.can_change_mode_class.
>
> So to prevent the canonicalization I reject integer modes when the sizes of to
> and from don't match and when we would have exited with false previously.
>
> This fixes all the ICEs and codegen regressions, but perhaps an x86 maintainer
> should take a deeper look at this hook implementation.
>
> Bootstrapped Regtested on x86_64-pc-linux-gnu and no issues.
>
> Ok for master?
>
> Thanks,
> Tamar
>
> gcc/ChangeLog:
>
>         * config/i386/i386.cc (ix86_can_change_mode_class): Update the target
>         hook.
>
> --- inline copy of patch --
> diff --git a/gcc/config/i386/i386.cc b/gcc/config/i386/i386.cc
> index c4d0e36e9c0a2256f5dde1f4dc021c0328aa0cba..477dd007ea80272680751b61e35cc3eec79b66c3 100644
> --- a/gcc/config/i386/i386.cc
> +++ b/gcc/config/i386/i386.cc
> @@ -19682,7 +19682,15 @@ ix86_can_change_mode_class (machine_mode from, machine_mode to,
>
>    /* x87 registers can't do subreg at all, as all values are reformatted
>       to extended precision.  */
> -  if (MAYBE_FLOAT_CLASS_P (regclass))
> +  if (MAYBE_FLOAT_CLASS_P (regclass)
> +      && VALID_FP_MODE_P (from)
> +      && VALID_FP_MODE_P (to))
> +    return false;
> +
> +  /* Reject integer modes if the sizes aren't the same.  It would have
> +     normally exited above.  */
> +  if (MAYBE_FLOAT_CLASS_P (regclass)
> +      && GET_MODE_SIZE (from) != GET_MODE_SIZE (to))
>      return false;
>
>    if (MAYBE_SSE_CLASS_P (regclass) || MAYBE_MMX_CLASS_P (regclass))
>
>
>
>
> --



-- 
BR,
Hongtao