[FFmpeg-devel] [PATCHv2 1/1] checkasm/lpc: test compute_autocorr

[James Almer]

On 12/18/2023 1:34 PM, Rémi Denis-Courmont wrote:
> Le sunnuntaina 17. joulukuuta 2023, 23.57.50 EET Martin Storsjö a écrit :
>>> Rounding errors would not cause a constant gap across the different test
>>> cases. This is most likely an off-by-one in the x86 code. I don't know if
>>> this is a bug in the x86 code, or the test case being a little loose with
>>> input parameters, and I have neither time, nor motivation not to mention
>>> skills to figure that out, so there will be no test cases for this
>>> function form me afterall.
>>
>> FWIW, we've had these situations elsewhere before as well, in swscale,
>> where the existing x86 assembly mismatches the C code in nontrivial ways,
>> and we have new assembly (aarch64 in that case) that is missing a test
>> (even if one was written) due to this.
>>
>> First I considered if we should collect these extra checkasm tests in some
>> branch somewhere, so they aren't lost, as they are useful when working on
>> assembly on other architectures.
>>
>> But rather than having the code rot, forgotten in a stray branch
>> somewhere, I wonder if we should just go ahead and merge it with an #if
>> !ARCH_X86 or something, together with a notable FIXME comment.
> 
> I'd certainly welcome more checkasm that literally anyone other than me wrote.
> If the divergence in the X86 code is simply due to optimising an inexact
> algorithm differently, that seems fine.
> 
> But if it is a case that the X86 code is demonstrably buggy, I think that it
> should be commented out or removed. That would not only fix a bug, but also put
> stronger incentives for X68 fanboys to actually fix it. Worst case, the
> optimisation has become meaningless and we have actually fixed a bug.

I looked at the sse2 implementation briefly and it may in fact be buggy.