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. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
