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

[Segher Boessenkool <segher@kernel.crashing.org>]

Hi!

On Sat, May 30, 2020 at 08:15:55AM +0100, Richard Sandiford wrote:
> Segher Boessenkool <segher@kernel.crashing.org> writes:
> >> Sure.  But the point is that FAILing isn't “explicitly allowed” for vcond*.
> >> In fact it's the opposite.

I disagree btw, and no one else has noticed for 16 years either.

In general, almost all patterns can FAIL, and those that can not are
simply because no one wrote fallback code.  Which means that all
targets that need a fallback need to implement the same thing for
themselves, which is just a waste and causes extra errors.

So, "cannot FAIL" should be a temporary thing, and should change to
"can FAIL" as soon as someone implements that, and never be changed
back -- and it should be how almost everything is in the first place
(and it still is, thankfully).

> > It has FAILed on rs6000 since 2004.
> 
> But that just means that the powerpc bug has been there since 2004,
> assuming these FAILs can actually trigger in practice.  At that time,
> the corresponding expand code was:

I, and I think most other people, thought it was allowed to FAIL (and
I still do).

> rtx
> expand_vec_cond_expr (tree vec_cond_expr, rtx target)

[ snip ]

So this was buggy.

> i.e. no fallbacks, and no checking whether the expansion even
> succeeded.  Since FAIL just causes the generator to return null,
> and since emit_insn is a no-op for null insns, the effect for
> FAILs was to emit no instructions and return an uninitialised
> target register.
> 
> The silent use of an uninitialised register was changed in 2011
> to an ICE, via the introduction of expand_insn.

Yeah, I ran into some of that in 2015, at least then not all of that
was fixed.  That was some very basic insn I think, that really should
never fail, a simple branch or something...  Was surprising though, a
good reminder to always check return values :-)

> The fact that we've had no code to handle the FAILs for 15+ years
> without apparent problems makes it even more likely that the FAILs
> never happen in practice.

AltiVec can do a lot less than VSX (and VSX on p7 can do less than on
p8, and that can do less than p9, etc.), so I am pretty certain it
could fail for some cases.  Only up to not so very long ago these
patterns were mainly (or only?) used via builtins, and the code for
those handles all those cases already.

> If you think the FAILs do trigger in practice, please provide an example.

As I said before, that is completely beside the point.

vcond is allowed to FAIL.  No pattern that can FAIL should ever be
changed to not allow that anymore.  This would make no sense at all.


Segher