Re: [PATCH] Change vcond<mode> to vcond<mode1><mode2>

[Uros Bizjak <ubizjak@gmail.com>]

On Fri, Sep 2, 2011 at 11:43 AM, Richard Guenther <rguenther@suse.de> wrote:

>> > > >> >> > Hmm.  But then I'd have to try emit an insn, right?  Currently
>> > > >> >> > the vectorizer simply looks for an optab handler ... the
>> > > >> >> > operands are not readily available (but their mode is known).
>> > > >> >> > So I'd create some fake regs, setup operands and call GEN_FCN
>> > > >> >> > on it?  If it succeds I'd have to delete emitted insns, etc.
>> > > >> >> > Or I could add a target hook ...
>> > > >> >>
>> > > >> >> Hm... indeed, too much complication...
>> > > >> >>
>> > > >> >> I'd say, let's go with modeless operands and a target hook. IMO, this
>> > > >> >> is much more flexible than checking optab for supported modes.
>> > > >> >> Existing way is appropriate for single mode patterns, but we have
>> > > >> >> interdependent modes here, at least on x86.
>> > > >> >>
>> > > >> >> The hook would have two input arguments, insn mode and compare mode,
>> > > >> >> where the hook returns suggested supported compare mode, or no mode,
>> > > >> >> if it really can't handle requested modes.
>> > > >> >
>> > > >> > I think a two mode vcond pattern is in fact much cleaner than
>> > > >> > a one mode + modeless pattern which gen* will complain about and
>> > > >> > a target hook.
>> > > >>
>> > > >> OK, but in this case, do not use mode iterators too much in order to
>> > > >> avoid invalid patterns.
>> > > >
>> > > > I don't see them as "invalid".  They will be unused (maybe combine
>> > > > would create them though?), but they have well-defined semantics
>> > > > with my proposed documentation.  And x86 can handle them just fine.
>> > >
>> > > OK, let's go this way then... We can clean up this later if at all.
>> >
>> > Certainly what I prefer (less work for me now) ;)  The smallest
>> > number of patterns would probably result from using vcond<mode><mode>
>> > to cover the same-mode cases and then add the 12 other patterns
>> > with the respective integer / float mode variant.  Thus we'd have
>> > 15 patterns in total (still much for my taste).
>> >
>> > Ideally we could have a mode attribute that would map possibly
>> > to an iterator, thus
>> >
>> > (define_mode_attr matching [(V4SF ["V4SF" "V4SI"]) (V8HI "V8HI") ...])
>> >
>> > or similar.  But I don't feel like adding this sort of mode
>> > attr that really is a hidden iterator ... ;)
>> >
>> > Thus, the following is the combined patch which bootstrapped and
>> > tested ok on x86_64-unknown-linux-gnu with {,-m32} over night,
>> > with the documentation for vcond added.
>> >
>> > Ok for trunk?
>>
>> I'm re-testing with the patterns having an extra condition like
>>    && (GET_MODE_NUNITS (<V_256:MODE>mode)
>>        == GET_MODE_NUNITS (<VF_256:MODE>mode))"
>> to have the HAVE_vcond* defines easily optimized.
>>
>> Ok?
>
> Ping.  I'd like to have approval for the x86 changes.

OK.

Thanks,
Uros.