Sep 28, 2022, 08:03 by r...@remlab.net: > Le 28 septembre 2022 00:32:42 GMT+03:00, Lynne <d...@lynne.ee> a écrit : > >Sep 27, 2022, 22:04 by r...@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-requ...@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. >
I think it's fine as-is. I'm sure we'll get more fate systems other than your U74 to check for that in the future. Patchset looks good to me. Will apply in a few hours. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
