Spencer Abson <spencer.ab...@arm.com> writes:
> On Fri, Jun 06, 2025 at 10:02:19AM +0100, Richard Sandiford wrote:
>> Spencer Abson <spencer.ab...@arm.com> writes:
>> > @@ -27292,10 +27291,16 @@ aarch64_emit_sve_invert_fp_cond (rtx target,
>> > rtx_code code, rtx pred,
>> > void
>> > aarch64_expand_sve_vec_cmp_float (rtx target, rtx_code code, rtx op0, rtx
>> > op1)
>> > {
>> > - machine_mode pred_mode = GET_MODE (target);
>> > machine_mode data_mode = GET_MODE (op0);
>> > + rtx pred = aarch64_sve_fp_pred (data_mode, nullptr);
>> >
>> > - rtx ptrue = aarch64_ptrue_reg (pred_mode);
>> > + /* The governing and destination modes. */
>> > + machine_mode pred_mode = GET_MODE (pred);
>> > + machine_mode target_mode = GET_MODE (target);
>> > +
>> > + /* Also determines SVE_KNOWN_PTRUE, since an unnatural GP from
>> > + sve_fp_pred would disable part of the operation. */
>> > + bool natural_p = pred_mode == target_mode;
>>
>> I'm not sure about calling this "unnatural". The SVE predicate system
>> was designed with this kind of flexibility in mind. How about:
>>
>> /* For partial vector modes, the choice of predicate mode depends
>> on whether we need to suppress exceptions for inactive elements.
>> If we do need to suppress exceptions, the predicate mode matches
>> the element size rather than the container size and the predicate
>> marks the upper bits in each container as inactive. The predicate
>> is then a ptrue wrt target_mode but not wrt pred_mode. It is the
>> ptrueness wrt pred_mode that matters here.
>>
>> If we don't need to suppress exceptions, the predicate mode matches
>> the container size, pred_mode == target_mode, and the predicate is
>> thus a ptrue wrt both target_mode and pred_mode. */
>> bool known_ptrue_p = pred_mode == target_mode;
>
> OK, I think referring to containers and elements is a good call. Maybe
> we ought to add a comment above the definition of VPRED, along the
> lines of:
>
> ;; For partial vector modes, this is the predicate mode associated
> ;; with the container size.
Yeah, sounds good to me.
> Then your suggestion for patch 06 sounds good too.
>
>>
>> There again, maybe my comment makes no sense to anyone other than me,
>> so please do push back if you have a better suggestion!
>
> Only that perhaps the last part of the first section could be:
>
> /* The predicate is then a ptrue wrt target_mode but not wrt pred_mode,
> it is the latter which matters here. */
Yeah, sounds good.
Thanks,
Richard
