Segher Boessenkool <seg...@kernel.crashing.org> writes:
> On Fri, May 29, 2020 at 06:26:55PM +0100, Richard Sandiford wrote:
>> Segher Boessenkool <seg...@kernel.crashing.org> writes:
>> > 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.
>
> 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:
/* Generate insns for VEC_COND_EXPR. */
rtx
expand_vec_cond_expr (tree vec_cond_expr, rtx target)
{
enum insn_code icode;
rtx comparison, rtx_op1, rtx_op2, cc_op0, cc_op1;
enum machine_mode mode = TYPE_MODE (TREE_TYPE (vec_cond_expr));
bool unsignedp = TYPE_UNSIGNED (TREE_TYPE (vec_cond_expr));
icode = get_vcond_icode (vec_cond_expr, mode);
if (icode == CODE_FOR_nothing)
return 0;
if (!target)
target = gen_reg_rtx (mode);
/* Get comparison rtx. First expand both cond expr operands. */
comparison = vector_compare_rtx (TREE_OPERAND (vec_cond_expr, 0),
unsignedp, icode);
cc_op0 = XEXP (comparison, 0);
cc_op1 = XEXP (comparison, 1);
/* Expand both operands and force them in reg, if required. */
rtx_op1 = expand_expr (TREE_OPERAND (vec_cond_expr, 1),
NULL_RTX, VOIDmode, 1);
if (!(*insn_data[icode].operand[1].predicate) (rtx_op1, mode)
&& mode != VOIDmode)
rtx_op1 = force_reg (mode, rtx_op1);
rtx_op2 = expand_expr (TREE_OPERAND (vec_cond_expr, 2),
NULL_RTX, VOIDmode, 1);
if (!(*insn_data[icode].operand[2].predicate) (rtx_op2, mode)
&& mode != VOIDmode)
rtx_op2 = force_reg (mode, rtx_op2);
/* Emit instruction! */
emit_insn (GEN_FCN (icode) (target, rtx_op1, rtx_op2,
comparison, cc_op0, cc_op1));
return target;
}
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.
>> 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.
>
> That is a bug in the callers then :-)
It was a bug in the powerpc patch you cited that added the FAILs
without changing target-independent code to cope with them.
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.
If you think the FAILs do trigger in practice, please provide an example.
Richard
