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. > > 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; This change looks reasonable since only VALID_FP_MODE_P will be allocated to FLOAT_CLASS. > + > + /* 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; Do you have a case(or a patch so I can reproduce the regression myself) to indicate the regression, so I can have a deep look. > > if (MAYBE_SSE_CLASS_P (regclass) || MAYBE_MMX_CLASS_P (regclass)) > > > > > --
-- BR, Hongtao
