Hi, On Sat, Jul 9, 2016 at 1:38 PM, James Almer <jamr...@gmail.com> wrote:
> On 5/8/2016 4:00 AM, Reimar Döffinger wrote: > > On 07.05.2016, at 02:56, Hendrik Leppkes <h.lepp...@gmail.com> wrote: > > > >> On Sat, May 7, 2016 at 2:02 AM, James Almer <jamr...@gmail.com> wrote: > >>> On 5/6/2016 8:48 PM, Timothy Gu wrote: > >>>> On Fri, May 06, 2016 at 12:08:14PM +0200, Hendrik Leppkes wrote: > >>>>> > >>>>> Just to document it, this has caused build breakage in various > >>>>> scenarios, even in GCC 5.3 (6.1 not tested). > >>>>> > >>>>> The latest reported on IRC just today here: > >>>>> libavcodec/sbrdsp.c: In function 'sbr_neg_odd_64_c': > >>>>> libavcodec/sbrdsp.c:47:13: internal compiler error: in > >>>>> vect_analyze_data_ref_accesses, at tree-vect-data-refs.c:2596 > >>>>> static void sbr_neg_odd_64_c(float *x) > >>>>> > >>>>> There are various other cases which usually involve inline asm when > >>>>> building with SIMD (ie. --cpu=host) and the optimizer running out of > >>>>> registers, for example: > >>>>> libavcodec/x86/cabac.h:192:5: error: 'asm' operand has impossible > constraints > >>>>> > >>>>> IMHO this feature is not quite ready to be enabled unconditionally in > >>>>> our code base, and we should re-evaluate this change. > >>>> > >>>> I don't have a problem with reverting this commit, but as James > mentioned I do > >>>> prefer the bug to be reported to GCC if possible. > >>>> > >>>> Have you also considered the possibility to enable this feature only > if inline > >>>> assembly is not enabled? > >>> > >>> Nobody disables inline asm when using GCC, so it'd be the same as > removing tree > >>> vectorization altogether to begin with. > >>> > >>> This feature gives some nice speed boost on parts of the code that > don't have > >>> hand written asm, so I'd very much rather keep it and try to get GCC > to fix bugs > >>> on their compilers. > >> > >> Fixing would be nice of course, but it should then only be enabled in > >> versions we know do not have problems, which is none right now. > > > > I had to disable this option for some unrelated projects at work, too, > since even with 5.2.1 (didn't retest with later) it miscompiled multiple > pieces of code. > > And that even on x86_64. > > I really think there still is an EXTREMELY high risk that it still will > frequently cause miscompiled code. > > Unless we actually get above 90% code coverage in FATE I just don't > think that is a reasonable option for us to ever enable in builds for users! > > Even for developer builds I have some doubts the speedup we might get at > some point is worth the time wasted debugging. > > If we have code parts with a significant speed advantage, enabling it > only for those and adding targeted tests would be an option. > > It also avoids the cases where this option can cause some (generally > very minor) slowdown due to vectorizing loops where it's not worth it. > > [14:31:33] <jamrial> > https://github.com/mpc-hc/mpc-hc/commit/fe1b4ebd1ab69109c898fd4aa250013e18d2d116 > looks like some projects using ffmpeg are disabling tree vectorization to > fix crashes > [14:33:03] <jamrial> guess we should disable it for good. it's brought > more problems than potential performance improvements > [14:35:19] <@nevcairiel> with gcc 6.1 it also doesnt build on windows > [14:35:27] <@nevcairiel> because inline asm and registers > [14:35:54] <@nevcairiel> (doesnt build on x86 if you use -msse or higher) > [14:36:05] <jamrial> yeah, happened to me > [14:36:22] <jamrial> -march=haswell on mingw x86_32 = cabac fails to > compile > [14:37:38] <jamrial> also with 5.x > > Not a lot of arguments remain to keep this in place, so i'll revert this > change soon if nobody objects. OK. Ronald _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
