[FFmpeg-devel] [PATCH 2/2] configure: Enable GCC vectorization on ≥4.9

[James Almer]

On 5/8/2016 4:00 AM, Reimar Döffinger wrote:
> On 07.05.2016, at 02:56, Hendrik Leppkes <h.leppkes at gmail.com> wrote:
> 
>> On Sat, May 7, 2016 at 2:02 AM, James Almer <jamrial at 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.