Re: [stage1][PATCH] Lower VEC_COND_EXPR into internal functions.

[Richard Biener <richard.guenther@gmail.com>]

On June 3, 2020 7:01:39 PM GMT+02:00, Segher Boessenkool <segher@kernel.crashing.org> wrote:
>Hi!
>
>On Wed, Jun 03, 2020 at 04:46:12PM +0200, Richard Biener wrote:
>> On Wed, Jun 3, 2020 at 4:17 PM David Edelsohn <dje.gcc@gmail.com>
>wrote:
>> > On Wed, Jun 3, 2020 at 9:41 AM Richard Sandiford
>> > <richard.sandiford@arm.com> wrote:
>> > > Well, it seems unfortunate to have to do that.
>> > >
>> > > I think Martin's powerpc patch is the correct one.
>
>It is papering over the issues a little -- the same assumption is made
>at lower levels as well, so all *that* needs to be changed as well (not
>"fixed", it is not a bug, we have a change in the vcond* interface
>here;
>oh and that should be documented as well).
>
>> > How about (3) help to remove reliance on this incorrect behavior
>from
>> > the PowerPC port?
>
>It is not a reliance on incorrect behaviour.  This is a change.  Which
>pretty much everyone seems to want, so fine, but that takes time.
>
>> > I didn't formally check, but if this is 16 years old, then it's
>from
>> > the original RHES Altivec work.
>
>It is, exactly.
>
>> > I don't believe that anyone fundamentally is objecting to "fixing
>this
>> > correctly".  I don't know the entire history of this discussion,
>but
>> > my objection is to a fix that breaks a long-time assumption of the
>> > PowerPC port and leaves it as an exercise to the PowerPC
>maintainers
>> > to fix it.
>
>*Exactly*.  This is changing an ancient interface, claiming "it always
>was that way" (which very obviously isn't true), and leaving the rs6000
>people to deal with all the fallout.  Again.
>
>> I _think_ there's nothing to fix besides removing the FAIL.
>
>All the lower levels need to get asserts as well.  We need a week or so
>to put the whole thing through the wringer.  The documentation needs to
>be changed by whoever changes the vcond* semantics.  All other ports
>should be checked, too.
>
>> And I would
>> have no idea how to "fix" the powerpc port here since a) we lack a
>testcase
>> that actually FAILs, b) I'm not familiar with the ISA.  So we did (3)
>by
>> replacing the FAILs with gcc_unreachable () and bootstrap/regtest
>this
>> without any regression which I think "proves" the failure modes do
>not
>> actually exist.
>
>Heh, assuming the testsuite is comprehensive?  Heh.  (Bootstrap doesn't
>mean much for vector code).
>
>> So I'm not sure how we can help.
>
>You'll have to document the "vcond* is not allowed to FAIL" change.
>We'll deal with the rest.  But testing needs a week or so.  (That is
>an extremely short timescale already).
>
>> A vcond can usually be emulated by vec_cmp plus masking.
>
>That would be the generic way to implement this of course, but
>apparently
>such code doesn't yet exist?  If there is a generic implementation it
>should be trivial to deal with FAILs.
>
>> So if
>> we ever get a testcase that runs into the gcc_unreachable () I'll
>promise
>> to fix it up using this strategy in the vcond expander.  But without
>a
>> testcase and powerpc ISA knowledge it's really hard.  Or do you want
>> us to stick the vec_cmp expansion fallback in place of the FAILs?
>> I'm sure the powerpc maintainers are better suited to do that even
>though
>> I'll probably manage with some cut&paste.  To recap: vcond is
>> equal to
>> 
>>   mask = vec_cmp of the comparison
>>   true_masked = true_op & mask;
>>   false_masked = false_op & ~mask;
>>   result = true_masked | false_masked;
>> 
>> but I believe this would be dead code never triggered.
>
>But that would be the generic code as well?  Is that not useful to have
>in any case?

Sure. If you remove the vcond patterns from your port the vectorizer will do this transparently for you. So if you do not actually have a more clever way of representing this in the ISA there's no point of the vcond patterns. (though I think the vec_cmp ones didn't originally exist) 

The point is the vectorizer relies on a optab query for querying backend support and power claims vcond support here. If you then FAIL you have lied. (not in your interpretation of the pattern docs but in the implementations since introduction of vcond named patterns) 

So if you're happy I'll document explicitly that vector named patterns may not FAIL. 

Richard. 

>
>Segher