Richard Biener <richard.guent...@gmail.com> writes: > On Tue, Jul 13, 2021 at 11:40 AM Richard Sandiford > <richard.sandif...@arm.com> wrote: >> >> Richard Biener <richard.guent...@gmail.com> writes: >> > On Tue, Jul 13, 2021 at 10:53 AM Kewen.Lin <li...@linux.ibm.com> wrote: >> >> >> >> Hi, >> >> >> >> When I added the support for Power10 newly introduced multiply >> >> highpart instrutions, I noticed that currently vectorizer >> >> doesn't try to vectorize multiply highpart pattern, I hope >> >> this isn't intentional? >> >> >> >> This patch is to extend the existing pattern mulhs handlings >> >> to cover multiply highpart. Another alternative seems to >> >> recog mul_highpart operation in a general place applied for >> >> scalar code when the target supports the optab for the scalar >> >> operation, it's based on the assumption that one target which >> >> supports vector version of multiply highpart should have the >> >> scalar version. I noticed that the function can_mult_highpart_p >> >> can check/handle mult_highpart well even without mul_highpart >> >> optab support, I think to recog this pattern in vectorizer >> >> is better. Is it on the right track? >> > >> > I think it's on the right track, using IFN_LAST is a bit awkward >> > in case yet another case pops up so maybe you can use >> > a code_helper instance instead which unifies tree_code, >> > builtin_code and internal_fn? >> > >> > I also notice that can_mult_highpart_p will return true if >> > only vec_widen_[us]mult_{even,odd,hi,lo} are available, >> > but then the result might be less optimal (or even not >> > handled later)? >> > >> > That is, what about adding optab internal functions >> > for [us]mul_highpart instead, much like the existing >> > ones for MULH{R,}S? >> >> Yeah, that's be my preference too FWIW. All uses of MULT_HIGHPART_EXPR >> already have to be guarded by can_mult_highpart_p, so replacing it with >> a directly-mapped ifn seems like a natural fit. (Then can_mult_highpart_p >> should be replaced with a direct_internal_fn_supported_p query.) > > But note can_mult_highpart_t covers use via > vec_widen_[us]mult_{even,odd,hi,lo} > but I think this specific pattern should key on [us]mul_highpart only? > > Because vec_widen_* implies a higher VF (or else we might miss vectorizing?)?
But wouldn't it be better to do the existing hi/lo/even/odd conversion in gimple, rather than hide it in expand? (Yes, this is feature creep. :-)) Richard