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
