diff mbox

[FFmpeg-devel] configure: disable the new optimizer in Visual Studio 2015 Update 3

Message ID CABPLASScSRDdvS3eczOSHZLE9tspEdZCMTDJ_qRb5yK9JFs+KA@mail.gmail.com
State New
Headers show

Commit Message

Kacper Michajłow Jan. 5, 2017, 11:11 p.m. UTC
2016-07-04 3:53 GMT+02:00 Kacper Michajlow <kasper93@gmail.com>:
> 2016-07-03 23:39 GMT+02:00 Hendrik Leppkes <h.leppkes@gmail.com>:
>> On Tue, Jun 28, 2016 at 12:01 PM, Hendrik Leppkes <h.leppkes@gmail.com> wrote:
>>> On Tue, Jun 28, 2016 at 11:48 AM, Hendrik Leppkes <h.leppkes@gmail.com> wrote:
>>>> Visual Studio 2015 Update 3 introduced a new SSA optimizer, however
>>>> it unfortunately causes miscompilations. Until it is fixed, the new
>>>> optimizations are disabled and should be re-checked on subsequent
>>>> compiler releases.
>>>>
>>>> Fixes recent FATE failure of fate-lavf-pam on VS2015.
>>>
>>> On that note, i'm not exactly sure which code actually miscompiles.
>>> I though its pamenc, but the generated files are identical, then I
>>> though its the crc computing code (crcenc/adler32), but those seem
>>> fine as well and would likely screw up in more then one case if they
>>> would miscompile.
>>>
>>> So that leaves pnmdec, I suppose, but I didn't make much headway to
>>> poke around in that. If anyone has any handy hints how to find the
>>> code in question that might be helpful.
>>> Maybe the code is actually not-quite-right and might enjoy some straigtening.
>>>
>>
>> Applied the patch to disable the new optimizations in MSVC as I did
>> not spot anything obviously "bad" in pamdec - which does not mean
>> there isn't anything there, of course.
>>
>
> This is pretty bad miscompilation in new SSA optimizer. It assumes
> that variable declared in loop doesn't change the value even if there
> is assignment. I minimized testcese and reported the bug
> https://connect.microsoft.com/VisualStudio/feedback/details/2890170
>
> - Kacper

Sorry for bumping such old thread, but I just got information that the
bug has been fixed in MSVC.

It is fixed in hotfix for VS2015 - KB3207317
https://support.microsoft.com/en-us/kb/3207317 and of course in
upcoming VS2017.

I've confirmed that this hotfix fixes fate-lavf-pam test. The
following change will enable SSA Optimizer on newer compiler versions.

---
 configure | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

 for pfx in "" host_; do

Comments

Michael Niedermayer Jan. 6, 2017, 12:04 p.m. UTC | #1
On Fri, Jan 06, 2017 at 12:11:57AM +0100, Kacper Michajlow wrote:
> 2016-07-04 3:53 GMT+02:00 Kacper Michajlow <kasper93@gmail.com>:
> > 2016-07-03 23:39 GMT+02:00 Hendrik Leppkes <h.leppkes@gmail.com>:
> >> On Tue, Jun 28, 2016 at 12:01 PM, Hendrik Leppkes <h.leppkes@gmail.com> wrote:
> >>> On Tue, Jun 28, 2016 at 11:48 AM, Hendrik Leppkes <h.leppkes@gmail.com> wrote:
> >>>> Visual Studio 2015 Update 3 introduced a new SSA optimizer, however
> >>>> it unfortunately causes miscompilations. Until it is fixed, the new
> >>>> optimizations are disabled and should be re-checked on subsequent
> >>>> compiler releases.
> >>>>
> >>>> Fixes recent FATE failure of fate-lavf-pam on VS2015.
> >>>
> >>> On that note, i'm not exactly sure which code actually miscompiles.
> >>> I though its pamenc, but the generated files are identical, then I
> >>> though its the crc computing code (crcenc/adler32), but those seem
> >>> fine as well and would likely screw up in more then one case if they
> >>> would miscompile.
> >>>
> >>> So that leaves pnmdec, I suppose, but I didn't make much headway to
> >>> poke around in that. If anyone has any handy hints how to find the
> >>> code in question that might be helpful.
> >>> Maybe the code is actually not-quite-right and might enjoy some straigtening.
> >>>
> >>
> >> Applied the patch to disable the new optimizations in MSVC as I did
> >> not spot anything obviously "bad" in pamdec - which does not mean
> >> there isn't anything there, of course.
> >>
> >
> > This is pretty bad miscompilation in new SSA optimizer. It assumes
> > that variable declared in loop doesn't change the value even if there
> > is assignment. I minimized testcese and reported the bug
> > https://connect.microsoft.com/VisualStudio/feedback/details/2890170
> >
> > - Kacper
> 
> Sorry for bumping such old thread, but I just got information that the
> bug has been fixed in MSVC.
> 
> It is fixed in hotfix for VS2015 - KB3207317
> https://support.microsoft.com/en-us/kb/3207317 and of course in
> upcoming VS2017.
> 
> I've confirmed that this hotfix fixes fate-lavf-pam test. The
> following change will enable SSA Optimizer on newer compiler versions.
> 
> ---
>  configure | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/configure b/configure
> index 398e843..cf82b3b 100755
> --- a/configure
> +++ b/configure
> @@ -6317,9 +6317,9 @@ EOF
>      check_func strtoll || add_cflags -Dstrtoll=_strtoi64
>      check_func strtoull || add_cflags -Dstrtoull=_strtoui64
>      # the new SSA optimzer in VS2015 U3 is mis-optimizing some parts
> of the code
> -    # this flag should be re-checked on newer compiler releases and put under a

patch is currupted by stray newline

[...]
diff mbox

Patch

diff --git a/configure b/configure
index 398e843..cf82b3b 100755
--- a/configure
+++ b/configure
@@ -6317,9 +6317,9 @@  EOF
     check_func strtoll || add_cflags -Dstrtoll=_strtoi64
     check_func strtoull || add_cflags -Dstrtoull=_strtoui64
     # the new SSA optimzer in VS2015 U3 is mis-optimizing some parts
of the code
-    # this flag should be re-checked on newer compiler releases and put under a
-    # version check once its fixed
-    check_cflags -d2SSAOptimizer-
+    # Issue has been fixed in MSVC v19.00.24218.
+    check_cpp_condition windows.h "_MSC_FULL_VER >= 190024218" ||
+        check_cflags -d2SSAOptimizer-
 fi