Code generation bug with new optimizer in Visual Studio 2015 Update 3

When I saw that Visual Studio 2015 Update 3 includes a brand new optimizer, my first thought was: yikes, I wonder how well this is tested. It can't be very mature code.

Well – it isn't. The following reproduces a code generation bug involving the optimizer and a lambda that captures local variables by reference:

 
  #include <iostream>
  #include <cstring>
  #include <conio.h>
  #include <windows.h>

  using namespace std;

  #define If(EXPR, T, A, B) \
      (([&]() -> T { if (EXPR) return (A); return (B); })())

  void Report(size_t x, size_t a, size_t b)
  {
      cout << x << endl;

           if (x == a && x > b) cout << "OK, result is strlen(argv[0])" << endl;
      else if (x == b && x > a) cout << "OK, result is strlen(argv[1])" << endl;
      else                      cout << "Error - result is garbage" << endl;
  }

  int main(int argc, char** argv)
  {
      if (argc < 2)
          cout << "Enter anything as first parameter" << endl
               << "If compiled with cl -EHsc, output will be "
                  "max(strlen(argv[0]), strlen(argv[1]))" << endl
               << "If compiled with cl -EHsc -O2 under VS 2015 Update 3 x64, "
                  "output will be a large garbage number" << endl
               << "If compiled with cl -EHsc -O2 under VS 2015 Update 3 x86, "
                  "output may or may not be a small garbage number" << endl;
      else
      {
          cout << "Opportunity to attach a debugger. "
                  "Press any key to continue" << endl;
          _getch();
          if (IsDebuggerPresent())
              DebugBreak();

          size_t a = strlen(argv[0]);
          size_t b = strlen(argv[1]);
          Report(If(a > b, size_t, a, b), a, b);
      }
      return 0;
  }

  /*

  Problem disassembly on x64:

  Optimizer appears to recognize that variables "a" and "b" never need to be
  stored on stack, so it gives them both a dummy stack location [rsp+40h].
  This location is never written to. But code generated for the If lambda
  returns values from the dummy stack location.

          size_t a = (size_t) strlen(argv[0]);
  00007FF60F43A41F  mov         rax,qword ptr [rbx]  
  00007FF60F43A422  or          r8,0FFFFFFFFFFFFFFFFh  
  00007FF60F43A426  mov         rdx,r8  
  00007FF60F43A429  nop         dword ptr [rax]  
  00007FF60F43A430  inc         rdx  
  00007FF60F43A433  cmp         byte ptr [rax+rdx],0  
  00007FF60F43A437  jne         main+0B0h (07FF60F43A430h)  
          size_t b = (size_t) strlen(argv[1]);
  00007FF60F43A439  mov         rax,qword ptr [rbx+8]  
  00007FF60F43A43D  nop         dword ptr [rax]  
  00007FF60F43A440  inc         r8  
  00007FF60F43A443  cmp         byte ptr [rax+r8],0  
  00007FF60F43A448  jne         main+0C0h (07FF60F43A440h)  
          Report(If(a > b, size_t, a, b), a, b);
  00007FF60F43A44A  cmp         rdx,r8  
  00007FF60F43A44D  lea         rax,[rsp+40h]  
  00007FF60F43A452  lea         rcx,[rsp+40h]  
  00007FF60F43A457  cmovbe      rcx,rax  
  00007FF60F43A45B  mov         rcx,qword ptr [rcx]  
  00007FF60F43A45E  call        Report (07FF60F4292F2h)  

  Example output:

  C:\Temp>VsUpdate3.exe a
  Opportunity to attach a debugger. Press any key to continue
  140694781775872
  Error - result is garbage

  */


Solution?

According to the MSDN blog post that introduces the new optimizer, it can be disabled using the undocumented flag:

  -d2SSAOptimizer-

The above snippet runs properly when compiled with this flag. The disassembly is:
  
        size_t a = strlen(argv[0]);
  00007FF7B48C9FAF  mov         rax,qword ptr [rbx]  
  00007FF7B48C9FB2  or          r8,0FFFFFFFFFFFFFFFFh  
  00007FF7B48C9FB6  mov         rdx,r8  
  00007FF7B48C9FB9  nop         dword ptr [rax]  
  00007FF7B48C9FC0  inc         rdx  
  00007FF7B48C9FC3  cmp         byte ptr [rax+rdx],0  
  00007FF7B48C9FC7  jne         main+0B0h (07FF7B48C9FC0h)  
        size_t b = strlen(argv[1]);
  00007FF7B48C9FC9  mov         rax,qword ptr [rbx+8]  
  00007FF7B48C9FCD  nop         dword ptr [rax]  
  00007FF7B48C9FD0  inc         r8  
  00007FF7B48C9FD3  cmp         byte ptr [rax+r8],0  
  00007FF7B48C9FD8  jne         main+0C0h (07FF7B48C9FD0h)  
        Report(If(a > b, size_t, a, b), a, b);
  00007FF7B48C9FDA  cmp         rdx,r8  
  00007FF7B48C9FDD  mov         rcx,r8  
  00007FF7B48C9FE0  cmova       rcx,rdx  
  00007FF7B48C9FE4  call        Report (07FF7B48B92F2h)  

Interestingly – the code looks cleaner and more optimized without the SSA Optimizer. :-)

Update blues

I first tried to work around this issue by downgrading back to Update 2. This is not for the faint-hearted:
  • After uninstalling Update 3, the development environment would no longer start; it would crash after displaying the splash screen.
  • The Microsoft pages that claim to offer the Update 2 installer perform a bait-and-switch, and link to the Update 3 installer instead.
  • I downloaded the full Update 2 ISO from MSDN Subscriber Downloads, only to find that it goes online, as well as consults locally cached data, and then also pushes Update 3.
This is after the Update 3 installer took 3 hours and 50 minutes to run, because it turns out it downloads 1.7 GB during the installation on-demand – and it does so much slower than it would have taken to download the full 6 GB ISO, all while preventing getting any work done due to installation in progress.

Fortunately, comments suggest the optimizer issue might be fixed within a week, so that this will no longer be an obstacle.

Comments

Richard Baranyi said…
Nice find, have you reported this issue to connect.microsoft.com?
denis bider said…
I have reported it using the built-in feedback feature.
Graphics Owl said…
Niceley spotted. I had the same bug a few days ago, but didn't have any time to look into it yet to create a test report. Thanks a lot.

About the downgrade. I had to fight the same issue too:
1) Uninstall VS
2) Use Iso with Update 2 and start the installer with /NoWeb /NoRefresh
3) Install, despite potential information that it will install update 3
4) Observe after the installation that the installer complains that it couldn't download update 3 (given it wanted to on the first page)
Andrew Pardoe said…
Thanks for the bug report. There are more comments on the Reddit thread (https://www.reddit.com/r/cpp/comments/4uzjab/code_generation_bug_with_new_optimizer_in_visual/) but the short story is that I've passed this bug report directly on to the dev responsible (who probably already has the Feedback item, but...)

Sorry if this is a double post. I never really figured out the Google+ stuff.
Lup Gratian said…
Hi,

I'm the main developer of the new optimizer.
This is a bug that has already beeen fixed. The fix will be part of the next VS micro-update, which will be released next week.

Thanks,
Gratian Lup
Visual C++ Optimizer team
denis bider said…
Hello Lup, Andrew -

I'm glad to hear the fix is already in the pipeline. I hope this is the only such issue we will encounter. :-)

Thank you for your replies!
Sergei Chernykh said…
Hello Lup, Andrew

What about this bug: https://connect.microsoft.com/VisualStudio/feedback/details/2988420/ssa-optimizer-fatal-error-c1063-when-compiling-simple-c-bit-manipulation-code

Is there any chance the fix will be in the next patch?
Lup Gratian said…
This comment has been removed by the author.
Lup Gratian said…
Hi Sergei,

Thanks for reporting the problem. It's a stack overflow caused by the Bit Estimator component, it will definitely be fixed, but can't tell yet in what micro-update release.

Thanks,
Gratian
Sergei Chernykh said…
Hello Lup,

Maybe I'm unlucky, but the very next project I tried to compile uncovered another bug: https://connect.microsoft.com/VisualStudio/feedback/details/2992985/ssa-optimizer-incorrect-integer-computation-results-with-when-optimizer-is-turned-on

Hope it will be fixed soon too :)
Lup Gratian said…
Hi Sergei,

This is exactly the same bug this blog post talks about.

Thanks,
Gratian
denis bider said…
I can confirm the bad code generation issue appears to be resolved after applying KB3165756.

Unfortunately, generated code remains worse in this case with the SSA Optimizer than without it. :(

After patch, with SSA Optimizer:

Report(If(a > b, size_t, a, b), a, b);
00007FF72B0DA44C cmp rdx,r8
00007FF72B0DA44F mov qword ptr [rsp+48h],r8
00007FF72B0DA454 lea rax,[b]
00007FF72B0DA459 lea rcx,[a]
00007FF72B0DA45E cmovbe rcx,rax
00007FF72B0DA462 mov rcx,qword ptr [rcx]
00007FF72B0DA465 call Report (07FF72B0C92F2h)

Whereas, SSA Optimizer disabled (-d2SSAOptimizer-):

Report(If(a > b, size_t, a, b), a, b);
00007FF7DB749FDA cmp rdx,r8
00007FF7DB749FDD mov rcx,r8
00007FF7DB749FE0 cmova rcx,rdx
00007FF7DB749FE4 call Report (07FF7DB7392F2h)
Lup Gratian said…
This comment has been removed by the author.
Lup Gratian said…
While solving the bug I also improved the code generation for this case and for some similar ones with float values, but the released fix doesn't include these. A future preview version of DEV15 will definitely have them and many other new improvements, before the final release.

Popular posts from this blog

"Unreachable" beauty standards

Is the internet ready for DMARC with p=reject?

When monospace fonts aren't: The Unicode character width nightmare