> But AArch64 does not return zero. It returns one of the operands of the
> conditional
> instruction.
>
> i.e. the implementation is
>
> static tree
> aarch64_preferred_else_value (unsigned, tree, unsigned int nops, tree *ops)
> {
> return nops == 3 ? ops[2] : ops[0];
> }
Ah, I should have checked :)
>
> So calling this requires me to already have created the 0 operand. Which is
> different
> than what RISC-V does which never inspects ops and returns a value based on
> the
> extension and mode.
>
> Don't think it would, ops here would be 4, mask, op1, op2, default.
> calling preferred_else_value on AArch64 would then return mask.
>
> Two callers of preferred_else_value always skip the mask operand, so
> They pass length -1 and &ops[1], e.g. gcc/tree-if-conv.cc and
> gcc/tree-vect-stmts.cc
>
> They would pass op1, op2, and we would return op1. Which is not a usable
> default value for comparisons on the target. As the target can only zero out
> inactive
> lanes in compares.
Ok, makes sense. That's where you were coming from when saying its usage was
open to interpretation ;)
So aarch64's hook implements that cond_add (or any cond_binop) overwrites the
first operand, while there are cond_ternops variants that overwrite a different
op each. The actual but implicit else policy is "unmodified". riscv, apart
from the FMAs, is not destructive in any source operand, just the destination
one, but still allows "unmodified" as well as "overwrite with ones".
A comparison is different and rather like a maskload in that regard (hence my
confusion) and that's why it doesn't really fit preferred_else_value.
I still would argue the preferred_else_value status quo is a somewhat
cumbersome and we should rather always pass a full cond_ifn, so the aarch64
hook could e.g. do
if (binop_ifn (...))
return ifn_get_first_op ()
else if (ternop_ifn)
...
else if (vec_cbranch_ifn (...))
return zeros;
That would work for tree-ifcvt and tree-vect-stmts but not directly for the
gimple-match-exports case where we just pass a list of ops with the mask/cond
being separate still.
I'm not sure this is in scope for your patch and I don't want to annoy you
(even more hehe) but, at least to me, it feels like code/contract clarification
to use preferred_else_value here.
Actually, when thinking about it, long-term we could even unify this with
maskload else-value handling which might be a bit over-engineered currently.
BTW I just noticed you didn't add bias operands to the new len optabs. It's
presumably unlikely that s390 or power want to support them (and not something
you wanted to hear ;) ) but when introducing the other len variants we agreed
to add the bias anyway for consistency.
Traditionally, we also had the argument order of (mask), else, len, bias so
add_mask_else_and_len_args can be used.
--
Regards
Robin
