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

[Michael Niedermayer]

On Thu, Oct 22, 2015 at 08:53:07AM -0400, Ganesh Ajjanagadde wrote:
> 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.

maybe ff_ctzll() was inlined in av_gcd and the compiler or debugger
then pointed to the wrong

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Those who are best at talking, realize last or never when they are wrong.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20151022/92a58227/attachment.sig>