Jakub Jelinek <ja...@redhat.com> writes:
> On Tue, Mar 05, 2019 at 08:53:00AM +0000, Richard Sandiford wrote:
>> Sorry, commented on the bug before seeing this patch.
>
> Sorry, I've committed before seeing your comment in the PR
> (and responded there after seeing it).
>
>> I don't think this is the way to go though. If we want match.pd
>> rules to have early checks for whether an ifn is supported, I think we
>> should get genmatch to do that itself rather than have to remember
>> to do it manually for each match.pd condition.
>
> Why? Most of the patterns don't introduce IFN_COND_* out of other code,
> just simplify existing IFN_COND_*. And those that adjust existing ones
> don't need these extra checks first.
>
>> In this case, isn't the underying problem that we only support some
>> vector conditions in VEC_COND_EXPRs and not as stand-alone comparisons?
>
> That is not a bug, I believe VEC_COND_EXPRs have been supported far earlier
> than those vec_cmp* optabs have been and VEC_COND_EXPRs is the only thing
> you want to use on targets which don't really have any mask registers for
> vectors, where the result of comparison is really vectorized x == y ? -1 : 0.
> vec_cmp* have been introduced for AVX512F I believe and are for the case
> when you store the result of the comparison in some bitset (mask), usually
> the mask has some other type than the vector it is comparing etc.
> (VECTOR_BOOLEAN_P at the GIMPLE type usually).
>
>> Shouldn't we address that directly and then treat the early checks as
>> a separate compile-time optimisation?
>>
>> As far as the patch itself goes, I don't understand why you have:
>>
>> internal_fn cond_fn = get_conditional_internal_fn (uncond_op); }
>>
>> when the cond_op iterator already gives the conditional internal function.
>
> I guess you're right and that could simplify the changes.
> That would be (untested except for make cc1):
LGTM, thanks. Given the discussion, I think it would also be worth having
a comment explaining why we're doing this, something like:
/* Avoid speculatively generating a stand-alone vector comparison
on targets that might not support them. Any target implementing
conditional internal functions must support the same comparisons
inside and outside a VEC_COND_EXPR. */
Richard
PS. Sorry for not commenting yesterday. I've now switched my
gcc.gnu.org email to my work address, so hopefully I'll be
a bit more responsive to bugzilla stuff in future.
