> -----Original Message----- > From: Robin Dapp <[email protected]> > Sent: 18 November 2025 10:57 > To: Tamar Christina <[email protected]>; Robin Dapp > <[email protected]>; Hongtao Liu <[email protected]> > Cc: [email protected]; nd <[email protected]>; [email protected]; Robin > Dapp <[email protected]> > Subject: Re: [PATCH v2 1/3]middle-end: support new > {cond{_len}_}vec_cbranch_{any|all} optabs [PR118974] > > > 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. >
100% Agree, I did think about it, and one way to do it is to change the interface from internal_fn to code_helper, which would allow me to pass down TREE EXPRs as well and then I could tell in the target what's being asked. Richi doesn't want IFNs inside gconds so that's why I materialize this change late during expand. > 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. > Yeah I'm selfishly trying to avoid updating the hook to avoid delaying this patch series. I can try to do it as a follow up possibly for next stage1 or now depending on what Richi thinks. But I definitely want to do it as a follow up. > 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. > I completely forgot about the bias, and think I misinterpreted VCOND_MASK_LEN. I was looking at this example https://godbolt.org/z/jofExx9q6 and was assuming that the _90 is the len and 0 is the bias? Is so I'll respin the patch with that adjustment. Thanks for the feedback! Tamar > -- > Regards > Robin
