[FFmpeg-devel] [PATCHv2] avutil/mathematics: speed up av_gcd by using Stein's binary GCD algorithm

[Ganesh Ajjanagadde]

On Thu, Oct 22, 2015 at 8:42 AM, Ganesh Ajjanagadde <gajjanag at mit.edu> wrote:
> On Thu, Oct 22, 2015 at 8:33 AM, Michael Niedermayer
> <michael at niedermayer.cc> wrote:
>> On Thu, Oct 22, 2015 at 07:04:41AM -0400, Ganesh Ajjanagadde wrote:
>>> On Thu, Oct 22, 2015 at 5:49 AM, Carl Eugen Hoyos <cehoyos at ag.or.at> wrote:
>>> > Ganesh Ajjanagadde <gajjanagadde <at> gmail.com> writes:
>>> >
>>> >> > This broke fate with -ftrapv
>>> >
>>> >> > The problem seems to be in av_gcd() and not the De-Bruijn version of
>>> >> > ff_ctzll since the gcc builtin is being used.
>>> >>
>>> >> Don't have the time to investigate right now - revert for now unless
>>> >> someone can fix it quickly.
>>> >
>>> > Ping?
>>> >
>>> > Please revert if nobody can fix this.
>>>
>>> "Fixing" this at the moment is highly dubious - read the thread. It
>>> does not even occur in a reproducible way, does not trigger on ubsan,
>>> I can't even see it on ftrapv. Show me why it is not a toolchain
>>> issue.
>>
>> IMO never just assume that a problem that appears after a change you do
>> is not caused by your code.
>> Because if you do and you are wrong the bug might never be fixed
>> while OTOH, if you assume its in your code you will sooner or later
>> by debuging find the bug or more details, and that may be a
>> code generation issue in the toolchain outside your code in case
>> you where wrong.
>> Its the safer default assumtation as the failure path if you are wrong
>> is less bad.
>>
>> either way, bug fixed
>> the reason for non reproduceablility likely was that the x86 asm
>> overrode the C code in which the bug was
>
> Ok, go ahead and revert. I just did not see the hurry for reverting;
> since it does not break normal builds.

Just saw your commit. I guess a lot of the confusion stemmed from the
fact that James's report was slightly misleading - it seemed to target
av_gcd as opposed to the De-Bruijn implementation, where it seems the
bug really was.

>
>>
>>
>> [...]
>> --
>> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>>
>> I have often repented speaking, but never of holding my tongue.
>> -- Xenocrates
>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>