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

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

On Thu, May 21, 2020 at 10:17 PM Segher Boessenkool
<segher@kernel.crashing.org> wrote:
>
> Hi!
>
> On Thu, May 21, 2020 at 03:29:49PM +0200, Martin Liška wrote:
> > Adding Segher to CC, he can help us.
>
> Oh dear.  Are you sure?
>
> > On 5/21/20 2:51 PM, Martin Liška wrote:
> > >Back to this I noticed that ppc64le target build is broken due to:
>
> > >insn-emit.o -MMD -MP -MF ./.deps/insn-emit.TPo insn-emit.c
> > >/home/marxin/Programming/gcc/gcc/config/rs6000/vector.md:357:11: error:
> > >vcondv4sfv4sf cannot FAIL
> > >   357 |     FAIL;
>
> Is it new that vcond cannot FAIL?  Because we have done that for years.
>
> Since this breaks bootstrap on a primary target, please revert the patch
> until it is sorted.
>
> > >which is caused by the 4 added optabs:
> > >
> > >+DEF_INTERNAL_OPTAB_FN (VCOND, 0, vcond, vec_cond)
> > >+DEF_INTERNAL_OPTAB_FN (VCONDU, 0, vcondu, vec_condu)
> > >+DEF_INTERNAL_OPTAB_FN (VCONDEQ, 0, vcondeq, vec_condeq)
> > >+DEF_INTERNAL_OPTAB_FN (VCOND_MASK, 0, vcond_mask, vec_cond_mask)
>
> > >looking at the generator:
>
> > >I get there due to:
> > >
> > >B- │516               if (find_optab (&p, XSTR (expand,
> > >0)))│
> > >    │517
> > > {                                   │
> > >    │518                   gcc_assert (p.op <
> > > NUM_OPTABS);   │
> > >    │519                   if
> > > (nofail_optabs[p.op])          │
> > >    │520                     can_fail_p =
> > > false;             │
> > >    │521
> > > }                                   │

OK, so this is an "artifact" of direct internal functions.  We do check that
expansion does not actually FAIL before emitting calls to those IFNs.

I guess this simply makes direct internal functions not a 100% match for
our use and the way out is to add regular internal functions mapping to
the optabs.  That is, I guess, for direct-internal functions it should be
enough to check direct_internal_function_supported_p which it is not
for the case of vcond*.

Richard, do you agree?

Thanks,
Richard.

> > >
> > >#define DEF_INTERNAL_OPTAB_FN(NAME, FLAGS, OPTAB, TYPE) \
> > >   nofail_optabs[OPTAB##_optab] = true;
>
> So yes it is new.  Please fix :-(
>
> > >Any hint what's bad? Note that x86_64-linux-gnu is fine.
> > >Do I miss a target hook?
>
> There is a new IFN that requires the existing optabs to never fail.  But
> they *do* sometimes fail.  That is what I understand from this anyway,
> please correct if needed :-)
>
> We can make the rs6000 patterns never FAIL if that is a good idea (I am
> not convinced however), but this should be documented, and all existing
> targets need to be checked.
>
> In general it is not pleasant at all to have patterns that cannot FAIL,
> it makes writing a (new) port much harder, and there can be cases where
> there is no sane code at all that can be generated for some cases, etc.
>
>
> Segher