Optimizer bug or threading bug?

[stratum]

The result is the same, 100 (because I still have tbb::atomic<double> as last_percent type).

[camsr]

Yes. There's no reassignment of last_percent if it's in the 0-100 range, unless there needs to be.
Otherwise you can use the if, else if, else nesting, and do the assignment in all 3 branches.

[stratum]

Yes. There's no reassignment of last_percent if it's in the 0-100 range, unless there needs to be.
Otherwise you can use the if, else if, else nesting, and do the assignment in all 3 branches.

There isn't any need for reassignment because it is assigned unconditionally just before that (and the value of prog_info.percentComplete is correct, the problem begins with last_percent).

Code: Select all

					last_percent = prog_info.percentComplete;
					if (last_percent < 0.0)
						last_percent = 0.0;
					if (last_percent > 100.0)
						last_percent = 100.0;

[camsr]

Yeah I think that making last_percent atomic was wrong. I haven't used it so I suggest checking out the documentation, there may be a member function or something that will work.

[camsr]

There was no warning about the incomplete logic block? What warnings if any is the compielr showing? :X

[stratum]

What is so weird about this is that the last_percent variable is local to the method, it is not being referenced by another thread, but marking it with volatile or tbb::atomic changes the result.

There was no warning about the incomplete logic block? What warnings if any is the compielr showing? :X

No because it's correct. If this was a functional language like lisp, I would say it needs to be complete but with c++ it's not the case. This code is pretty trivial and the compiler gets the result wrong.

[stratum]

[deleted]

I have given up trying to make sense of this.
Apparently it's April 1 according to the date of the compiler.

[mystran]

As "expected" this one gives correct behavior:

Code: Select all

					if (last_percent < 0.0)
						last_percent = 0.01;
					else if (last_percent > 100.0)
						last_percent = 100.0;

Ok, so you have a compiler bug then.

[stratum]

As "expected" this one gives correct behavior:

Code: Select all

					if (last_percent < 0.0)
						last_percent = 0.01;
					else if (last_percent > 100.0)
						last_percent = 100.0;

Ok, so you have a compiler bug then.

Who knows, the only thing I was able do with shorter code fragments extracted from the same code was to cause cl.exe to crash and couldn't reproduce the problem itself.

[quikquak]

So all values between 0.0 and .01 pass straight through?

[stratum]

It acts somewhat like as if the code is:

Code: Select all

               if (some_silly_mysterious_value_dependent_upon_last_percent_declaration_properties)
                  last_percent = 0.0;
               else 
                  last_percent = 100.0;

[stratum]

BTW I have tried an address sanitizer (intel inspector) on the code and the most serious error it has found was a memory leak in code generated by GSOAP among many false positives about Qt allocator usage (new/delete mismatch) and similar other things which are found in every process when such a tool is used). That was with the "RelWithDebInfo" cmake configuration though (which is basically /O0 with release c++ runtime and debug information), and that configuration works correctly anyway.

[camsr]

After reading the tbb::atomic documentation page and doing some searches on using this template with floating point datatypes, maybe the problem is actually that an atomic float (in tbb) isn't well defined. You could use an atomic pointer maybe, or recast m_percentage to an integer and use integer operations... at least where it appears as an atomic.

[stratum]

After reading the tbb::atomic documentation page and doing some searches on using this template with floating point datatypes, maybe the problem is actually that an atomic float (in tbb) isn't well defined. You could use an atomic pointer maybe, or recast m_percentage to an integer and use integer operations... at least where it appears as an atomic.

Unfortunately it's not that. When the problem was first seen that code didn't have any tbb::atomic variables in it. The local variable last_percent is not accessed by any other thread. m_Percentage is, but even if it was not guarded by tbb::atomic I would not expect the result to be 0 or 100 consistently, it could possibly be an incorrect value with a very low probability, but it is consistently wrong and the error starts around the if block regarding last_percent limit checks. This code works fine with gcc (both debug and optimized builds), and unoptimized builds of visual studio are OK too, and intel inspector (on unoptimized builds) does not find anything interesting regarding memory usage. Thread checking may tell another story but it's too slow to use and I couldn't make it yet work because of that.

[camsr]

It seems the safest option then is to disable the optimization. It would probably not impact performance in any notable way. Whether or not this is a bug with MSVC would require a much deeper look into the current PE.