Segher Boessenkool <seg...@kernel.crashing.org> writes:
> On Fri, May 29, 2020 at 05:57:13PM +0100, Richard Sandiford wrote:
>> Segher Boessenkool <seg...@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
