Re: [FFmpeg-devel] [RFC] [PATCH 0/7] RISC-V V vector length dealings

["Rémi Denis-Courmont" <remi@remlab.net>]

Le 28 septembre 2022 00:32:42 GMT+03:00, Lynne <dev@lynne.ee> a écrit :
>Sep 27, 2022, 22:04 by remi@remlab.net:
>
>> Hello,
>>
>> As a general rule, scalable vector instruction sets should be used with the
>> largest possible vector length. There are however a number of operations that
>> just happen with a fixed size, and this patchset exhibits the simplest one I
>> could find. The proper RISC-V Vector extension guarantees a minimum vector
>> length of 128 bits. In theory though the underlying specification also allows
>> for (embedded designs with) only 32 or 64 bits per vector.
>>
>> The RFC is how should this be dealt with? The simplest possibibility is to
>> simply assume 128 bits. Indeed, I am not aware of any actual or proposed
>> processor IP with shorter-than-128-bit vectors, even less so, one upon which
>> FFmpeg would be used. For what it is worth, ARM SVE guarantees a minimum of
>> 128 bits per vector too. In that case, we can drop the first patch, and
>> simplify the following ones.
>>
>> Another option is to expose the vector length via the CPU flags as proposed
>> earlier by Lynne. Though this is unorthodox the vector length is not a proper
>> flag. The vector length can readily be retrieved from a read-only unprivileged
>> CSR, and this patchset instead introduces a simple inline wrapper therefore.
>> The downside of this approach is that this is nominally undefined behaviour,
>> and typically will raise a SIGILL, if the processor does not support the
>> vector extension.
>>
>
>Where's the undefined behavior? If it's guarded by an if, the
>function will return the maximum length. I don't mind that it's not
>a cpuflag.
>
>_______________________________________________
>ffmpeg-devel mailing list
>ffmpeg-devel@ffmpeg.org
>https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>To unsubscribe, visit link above, or email
>ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>

The UB occurs if the helper is called on a CPU without vectors. There is no (I think) UB in the patchset. My point is rather that it might be prone to programming errors, not that they would be actual errors already.

Presumably someone could accidentally insert or move a call to ff_rv_get_vlenb() before the proper CPU flags check, and not notice that it would cause UB on some CPUs.

With that said, if there are no objections to the approach in this series, I'm obviously fine with that.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".