Updated patch 2 with explanation included in commit message and changes requested.
Bootstrapped and regression tested on aarch64 > -----Original Message----- > From: Joel Hutton > Sent: 12 November 2021 11:42 > To: Richard Biener <rguent...@suse.de> > Cc: gcc-patches@gcc.gnu.org; Richard Sandiford > <richard.sandif...@arm.com> > Subject: RE: [vect-patterns] Refactor widen_plus/widen_minus as > internal_fns > > > please use #define INCLUDE_MAP before the system.h include instead. Done. > > Is it really necessary to build a new std::map for each optab lookup?! > > That looks quite ugly and inefficient. We'd usually - if necessary at > > all - build a auto_vec<std::pair<key,data> > and .sort () and .bsearch () > > it. > Ok, I'll rework this part. In the meantime, to address your other comment. Done. > > I'm not sure I understand DEF_INTERNAL_OPTAB_MULTI_FN, neither this > > cover letter nor the patch ChangeLog explains anything. > > I'll attempt to clarify, if this makes things clearer I can include this in > the > commit message of the respun patch: > > DEF_INTERNAL_OPTAB_MULTI_FN is like DEF_INTERNAL_OPTAB_FN except it > provides convenience wrappers for defining conversions that require a hi/lo > split, like widening and narrowing operations. Each definition for <NAME> > will require an optab named <OPTAB> and two other optabs that you specify > for signed and unsigned. The hi/lo pair is necessary because the widening > operations take n narrow elements as inputs and return n/2 wide elements > as outputs. The 'lo' operation operates on the first n/2 elements of input. > The 'hi' operation operates on the second n/2 elements of input. Defining an > internal_fn along with hi/lo variations allows a single internal function to > be > returned from a vect_recog function that will later be expanded to hi/lo. > > DEF_INTERNAL_OPTAB_MULTI_FN is used in internal-fn.def to register a > widening internal_fn. It is defined differently in different places and > internal- > fn.def is sourced from those places so the parameters given can be reused. > internal-fn.c: defined to expand to hi/lo signed/unsigned optabs, later > defined to generate the 'expand_' functions for the hi/lo versions of the fn. > internal-fn.def: defined to invoke DEF_INTERNAL_OPTAB_FN for the original > and hi/lo variants of the internal_fn > > For example: > IFN_VEC_WIDEN_PLUS -> IFN_VEC_WIDEN_PLUS_HI, > IFN_VEC_WIDEN_PLUS_LO > for aarch64: IFN_VEC_WIDEN_PLUS_HI -> vec_widen_<su>addl_hi_<mode> > -> (u/s)addl2 > IFN_VEC_WIDEN_PLUS_LO -> vec_widen_<su>addl_lo_<mode> > -> (u/s)addl > > This gives the same functionality as the previous > WIDEN_PLUS/WIDEN_MINUS tree codes which are expanded into > VEC_WIDEN_PLUS_LO, VEC_WIDEN_PLUS_HI. > > Let me know if I'm not expressing this clearly. > > Thanks, > Joel
0001-vect-patterns-Refactor-to-allow-internal_fn-s.patch
Description: 0001-vect-patterns-Refactor-to-allow-internal_fn-s.patch
0002-vect-patterns-Refactor-widen_plus-as-internal_fn.patch
Description: 0002-vect-patterns-Refactor-widen_plus-as-internal_fn.patch
0003-Remove-widen_plus-minus_expr-tree-codes.patch
Description: 0003-Remove-widen_plus-minus_expr-tree-codes.patch
