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

[Richard Sandiford <richard.sandiford@arm.com>]

Segher Boessenkool <segher@kernel.crashing.org> writes:
> On Fri, May 29, 2020 at 05:57:13PM +0100, Richard Sandiford wrote:
>> Segher Boessenkool <segher@kernel.crashing.org> writes:
>> > On Fri, May 29, 2020 at 02:17:00PM +0200, Richard Biener wrote:
>> >> Now it looks like that those verification also simply checks optab
>> >> availability only but then this is just a preexisting issue (and we can
>> >> possibly build a testcase that FAILs RTL expansion for power...).
>> >> 
>> >> So given that this means the latent bug in the powerpc backend
>> >> should be fixed and we should use a direct internal function instead?
>> >
>> > I don't see what you consider a bug in the backend here?  The expansion
>> > FAILs, and it is explicitly allowed to do that.
>> 
>> Well, the docs say:
>> 
>>   …  For **certain** named patterns, it may invoke @code{FAIL} to tell the
>>   compiler to use an alternate way of performing that task.  …
>> 
>> (my emphasis).  Later on they say:
>> 
>>   @findex FAIL
>>   @item FAIL
>>   …
>> 
>>   Failure is currently supported only for binary (addition, multiplication,
>>   shifting, etc.) and bit-field (@code{extv}, @code{extzv}, and @code{insv})
>>   operations.
>> 
>> which explicitly says that vcond* isn't allowed to fail.
>> 
>> OK, so that list looks out of date.  But still. :-)
>> 
>> We now explicitly say that some patterns aren't allowed to FAIL,
>> which I guess gives the (implicit) impression that all the others can.
>> But that wasn't the intention.  The lines were just added for emphasis.
>> (AFAIK 7f9844caf1ebd513 was the first patch to do this.)
>
> Most patterns *do* FAIL on some target.  We cannot rewind time.

Sure.  But the point is that FAILing isn't “explicitly allowed” for vcond*.
In fact it's the opposite.

If we ignore the docs and look at what the status quo actually is --
which I agree seems safest for GCC :-) -- then patterns are allowed to
FAIL if target-independent code provides an expand-time fallback for
the FAILing case.  But that isn't true for vcond either.
expand_vec_cond_expr does:

  icode = get_vcond_icode (mode, cmp_op_mode, unsignedp);
  if (icode == CODE_FOR_nothing)
    ...

  comparison = vector_compare_rtx (VOIDmode, tcode, op0a, op0b, unsignedp,
				   icode, 4);
  rtx_op1 = expand_normal (op1);
  rtx_op2 = expand_normal (op2);

  create_output_operand (&ops[0], target, mode);
  create_input_operand (&ops[1], rtx_op1, mode);
  create_input_operand (&ops[2], rtx_op2, mode);
  create_fixed_operand (&ops[3], comparison);
  create_fixed_operand (&ops[4], XEXP (comparison, 0));
  create_fixed_operand (&ops[5], XEXP (comparison, 1));
  expand_insn (icode, 6, ops);
  return ops[0].value;

which ICEs if the expander FAILs.

So whether you go from the docs or from what's actually implemented,
vcond* isn't currently allowed to FAIL.  All Richard's gcc_unreachable
suggestion would do is change where the ICE happens.

Richard