On Tue, Nov 15, 2022 at 4:51 PM Tamar Christina <tamar.christ...@arm.com> wrote: > > > -----Original Message----- > > From: Hongtao Liu <crazy...@gmail.com> > > Sent: Tuesday, November 15, 2022 8:36 AM > > To: Tamar Christina <tamar.christ...@arm.com> > > Cc: Richard Sandiford <richard.sandif...@arm.com>; Tamar Christina via > > Gcc-patches <gcc-patches@gcc.gnu.org>; nd <n...@arm.com>; > > rguent...@suse.de > > Subject: Re: [PATCH 3/8]middle-end: Support extractions of subvectors from > > arbitrary element position inside a vector > > > > Hi: > > I'm from https://gcc.gnu.org/pipermail/gcc-patches/2022- > > November/606040.html. > > > } > > > > > > /* See if we can get a better vector mode before extracting. */ > > > diff --git a/gcc/optabs.cc b/gcc/optabs.cc index > > > > > cff37ccb0dfc3dd79b97d0abfd872f340855dc96..f338df410265dfe55b689616009 > > 0 > > > a453cc6a28d9 100644 > > > --- a/gcc/optabs.cc > > > +++ b/gcc/optabs.cc > > > @@ -6267,6 +6267,7 @@ expand_vec_perm_const (machine_mode mode, > > rtx v0, rtx v1, > > > v0_qi = gen_lowpart (qimode, v0); > > > v1_qi = gen_lowpart (qimode, v1); > > > if (targetm.vectorize.vec_perm_const != NULL > > > + && targetm.can_change_mode_class (mode, qimode, ALL_REGS) > > It looks like you want to guard gen_lowpart, shouldn't it be better to use > > validate_subreg or (tmp = gen_lowpart_if_possible (mode, target_qi)). > > IMHO, targetm.can_change_mode_class is mostly used for RA, but not to > > guard gen_lowpart. > > Hmm I don't think this is quite true, there are existing usages in expr.cc > and rtanal.cc > That do this and aren't part of RA. As I mentioned before for instance the > canoncalization of vec_select to subreg in rtlanal for instances uses this. In theory, we need to iterate through all reg classes that can be assigned for both qimode and mode, if any regclass returns true for targetm.can_change_mode_class, the bitcast(validate_subreg) should be ok. Here we just passed ALL_REGS. > > So there are already existing precedence for this. And the documentation for > the hook says: > > "This hook returns true if it is possible to bitcast values held in registers > of class rclass from mode from to mode to and if doing so preserves the > low-order bits that are common to both modes. The result is only meaningful > if rclass has registers that can hold both from and to. The default > implementation returns true" > > So it looks like it's use outside of RA is perfectly valid.. and the > documentation also mentions > in the example the use from the mid-end as an example. > > But if the mid-end maintainers are happy I'll use something else. > > Tamar > > > I did similar things in > > https://gcc.gnu.org/pipermail/gcc-patches/2021-September/579296.html > > (and ALL_REGS doesn't cover all cases for registers which are both available > > for qimode and mode, ALL_REGS fail doesn't mean it can't be subreg, it just > > means parts of ALL_REGS can't be subreg. but with a subset of ALL_REGS, > > there could be a reg class which return true for > > targetm.can_change_mode_class) > > > && targetm.vectorize.vec_perm_const (qimode, qimode, target_qi, > > v0_qi, > > > v1_qi, qimode_indices)) > > > return gen_lowpart (mode, target_qi); @@ -6311,7 +6312,8 @@ > > > expand_vec_perm_const (machine_mode mode, rtx v0, rtx v1, > > > } > > > > > > if (qimode != VOIDmode > > > - && selector_fits_mode_p (qimode, qimode_indices)) > > > + && selector_fits_mode_p (qimode, qimode_indices) > > > + && targetm.can_change_mode_class (mode, qimode, ALL_REGS)) > > > { > > > icode = direct_optab_handler (vec_perm_optab, qimode); > > > if (icode != CODE_FOR_nothing) > > > diff --git a/gcc/testsuite/gcc.target/aarch64/ext_1.c > > > b/gcc/testsuite/gcc.target/aarch64/ext_1.c > > > new file mode 100644 > > > index > > > > > 0000000000000000000000000000000000000000..18a10a14f1161584267a8472e5 > > 71 > > > b3bc2ddf887a > > > > > > > > > > -- > > BR, > > Hongtao
-- BR, Hongtao
