ThePPK

Hello colleagues!
We've recently faced a very strange behavior of VS2005 compiler optimization. The following code shows the problem which seems to be both very intricate and unstable. We've tried the code on Athlon, P4 and Core Duo processors and VS2005 Pro, Std and Express with the same result. The code should be used in a newly created Windows console app in C++. The problem occurs only under Release configuration.

#include "stdafx.h"

//simple structure for holding a rectangle
typedef struct MyRect
{
int x;
int y;
int width;
int height;
}
MyRect;

//create a new MyRect structure
MyRect myRect(int x, int y, int width, int height)
{
MyRect r;

r.x = x;
r.y = y;
r.width = width;
r.height = height;

return r;
}

//simple structure for holding a rectangle with floating point coords
typedef struct MyFloatRect
{
float x;
float y;
float width;
float height;
}
MyFloatRect;

//a function to perform coordinates shifting
MyRect ShiftRect(const MyFloatRect& box)
{
float x = box.x;
float y = box.y;
float width = box.width;
float height = box.height;
printf("\n%f %f %f %f", box.x, box.y, box.width, box.height);

return myRect((int)(x - width / 2.0f), (int)(y - height / 2.0f), (int)width, (int)height);
}

int _tmain(int argc, _TCHAR* argv[])
{
MyFloatRect box;
box.x = 40.0f;
box.y = 30.0f;
box.width = 20.0f;
box.height = 10.0f;

MyRect rect = ShiftRect(box);
printf("\n%d %d %d %d", rect.x, rect.y, rect.width, rect.height);
}

If you perform calculations manually, you'll see, that normal output should be as follows:
40.000000 30.000000 20.000000 10.000000
30 25 20 10
But the program will show you:
40.000000 30.000000 20.000000 10.000000
39 -170 10 10

You can now see an absolutely strange behavior that can be cured in 7 (so far known) ways:
1. Create an empty destructor for MyRect structure (what a nonsense!)
2. Remove printf from ShiftRect() function
3. Compile in Debug
4. Use /Od compiler key to disable optimization
5. Change 2.0f constant in one of two usages at myRect() call to, say, 1.0f (strange!)
6. Use "(int)width / 2.0f" instead of "width / 2.0f" int myRect() call
7. Use /fp:fast for fast floating point model
8. etc

Are we doing something wrong or is that a bug in the compiler


Re: Visual C++ Language Code optimization bug in VS2005

einaros

Yeah, I see even more workarounds, but nothing apparently wrong with your approach. With a bug such as this it's weird if it hasn't already been spotted and reported, but at the same time; the set of prerequisite may be complicated enough for others not to have seen this exact problem yet.

Go to http://connect.microsoft.com/VisualStudio and see if you can find something similar to this reported. I have a hard time suggesting exactly what to search for, so you should probably just try optimization in combination with floats, and then go right ahead and report the problem as reproduced in this post.

When you've submitted it, please paste the resulting link back here, for future reference.






Re: Visual C++ Language Code optimization bug in VS2005

JeroGrav

myRect() looks like it could use some improvement. Someone smarter than me might be able to confirm this but what myRect is doing is:

1) create empty MyRect (r)

2) populate r

3) take a copy of r and return it

4) destroy r

While all that seems to be ok, it also seems a little dangerous to me. My guess is that somewhere along the line, the values in the MyRect structure aren't getting copied across properly.






Re: Visual C++ Language Code optimization bug in VS2005

einaros

JeroGrav wrote:

While all that seems to be ok, it also seems a little dangerous to me. My guess is that somewhere along the line, the values in the MyRect structure aren't getting copied across properly.

There's nothing dangerous about that. Shallow copies are used just about everywhere, and also frequently optimized away completely by the compiler, through inlining and (named) return value optimization.






Re: Visual C++ Language Code optimization bug in VS2005

aao123

Nope, I could not find anything wrong with your code. If you insert extra print statement bug is gone. If you use double instead of float bug is gone. I would say it is compiler team's bug. I am not expert but looking at disassembly bug is exactly at

return myRect((int)(x - width / 2.0f), (int)(y - height / 2.0f), (int)width, (int)height);

Wow that is one scary bug.





Re: Visual C++ Language Code optimization bug in VS2005

Brian Kramer

If performance is not an issue, a good workaround is to use #pragma optimize("",off) and then #pragma optimize("',on") around the affected code.



Re: Visual C++ Language Code optimization bug in VS2005

ThePPK

Hello!

We've completed a survey on the topic. The results are:
1. as mentioned at http://forums.microsoft.com/MSDN/ShowPost.aspx PostID=125048&SiteID=1 a new __ftol2_sse instruction for converting float to int is used in VS2005
2. some people've faced problems with new float-to-int conversion
3. a MS representative in the aforesaid post admitted that "We get these functions from Intel and getting documentation on them appears to be difficult" so no one knows if this instruction works properly
4. it is a complex behaviour of several superposable reasons (for example the code doesn't work if you disable ShiftRect() inlining using __declspec(noinline) and remove printf() at the same time)

I'm going to post the report as einaros
suggested. And I'll definetely post the link here.
Actually I'm using Intel's OpenCV library that is based on floats rather than doubles. Of course I can't just replace float with double in the entire 3rd party library. Along with that I can expect that similar or even more complicated bugs will arise crashing our program at any unpredictable moment. That's why I can't agree on the solution proposed by Brian Kramer not only because I need rapid calculations but also because I consequently should wrap in those pragmas all functions that use floating point. And that accounts for about 40% of code!

We'll continue the survey and post the results here.





Re: Visual C++ Language Code optimization bug in VS2005

Christopher Prest

"myRect((int)(x - width / 2.0f), (int)(y - height / 2.0f), (int)width, (int)height);"


Im not 100% sure but you mixed int with float. during optimization you may see an error. Even tho its casted at the end, the code will perform a int vs Float operation before the cast took place.




Re: Visual C++ Language Code optimization bug in VS2005

Brian Kramer

ThePPK wrote:

That's why I can't agree on the solution proposed by Brian Kramer not only because I need rapid calculations but also because I consequently should wrap in those pragmas all functions that use floating point. And that accounts for about 40% of code!

I wasn't suggesting using #pragma optimize around all code that uses floating point, just around the function where the compiler is generating incorrect code.

Definitely get a bug report filed so that the compiler backend developer can understand it and give you a more targeted workaround. Compiler bugs are hard to characterize when you're only looking at it from the outside.

Brian





Re: Visual C++ Language Code optimization bug in VS2005

ThePPK

But I just can't use pragmas "around the function where the compiler is generating incorrect code" and be sure that no other code generates the same or even worse error. Of course the solution will help me to fix the bug in this very function. But I just can't imagine what might happen in other functions making me doubt stability of my software.

Unfortunatelly I can't submit the bug at https://connect.microsoft.com/VisualStudio/Feedback because I don't have necessary permissions. I'll be very grateful if anybody knows how to get them.




Re: Visual C++ Language Code optimization bug in VS2005

Brian Kramer

I verified that this is a bug with how the compiler uses _ftol2_sse.  It looks as though the compiler backend is not doing the correct register bookkeeping in the compilation of ShiftRect, based on the fact that the printf is needed for this bug to be symptomatic.

The VC8 compiler is quite stable and from being in this forum for two years and opening tons of frivolous front end bugs and not seeing any backend bugs in native C++ code generation, I can attest to the frequency of these kind of bugs being miniscully low.   I say this not to defend Microsoft's compiler but to temper your worries about how likely you're to hit this sort of thing.  But regardless of the odds, working around the issue by either perturbing the code (e.g. use volatile modifier inside ShiftRect), or #pragma around ShiftRect is the right thing to do because it makes your code more correct.  Whether or not this bug can manifest elsewhere is orthogonal to fixing the cases you do know about. 

Also, based on what I see, I doubt you will see any measurable perf impact from the #pragma unless you're calling ShiftRect in some kind of tight loop for a lot of iterations.

Or, if you don't mind a warning that you can't disable, you can throw the deprecated flag /QIfist (Q capital "eye" fist), which disables the use of _ftol2_sse.  This tells the compiler to use the fistp conversion function instead of the _ftol2_sse intrinsic.  I don't know the details of how these are different.

Or, you can switch to a different compiler, but that will probably result in a bigger development issue.

I opened a bug on your behalf, link here.   Note that I condensed your code a bit, and used the more traditional C++ construction of your rects.

Hope this helps,

Brian





Re: Visual C++ Language Code optimization bug in VS2005

ThePPK

Thank you, Brian! I do appreciate you helped me with the submission of the bug. Let us see what they answer.
As for your suggestion to use /QIfist I'm afraid that is not suitable. The matter is that fistp instruction performs "ambient rounding mode" (MSDN 2005), so it'll produce 1 int out of 0.6f. On the contrary _ftol2_sse instruction works as a regular (int) cast should work (without rounding) producing 0 int out of 0.6f.

Of course I need regular behaviour of the (int) cast.




Re: Visual C++ Language Code optimization bug in VS2005

Brian Kramer

Thanks for the explanation. Can't you bias the float before rounding it by subtracting 0.5 0.6 - 0.5 = 0.1 -> fistp -> 0, and 1.2 - 0.5 = 0.7 -> fistp ->1.0. (Negative cases won't apply in your case.) If this works, you can #ifdef the old and new version for safekeeping.





Re: Visual C++ Language Code optimization bug in VS2005

ThePPK

Thank you again, Brian! Relying on your experience I think it's really an extraordinary coincidence to find a bug like this. So I should not be so scared that it could appear again elsewhere. Though the software performs millions ShiftRect() calls it's insignificant to the overall computational complexity, so I suppose your approach will suit us.
But nevertheless I'll keep track of the bug report you've kindly posted for me (by the way, thanks for clarifying the code and making it shorter). As something helpful appears I'll post it here.