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