4 ms·

Unless 'launched' was directly or indirectly local to the stack frame, I don't see how this optimisation is legal. In general your compiler has to assume values

by nly 13y ago

Unless 'launched' was directly or indirectly local to the stack frame, I don't see how this optimisation is legal. In general your compiler has to assume values will change across an opaque function call.

to3m 13y ago
It presumably checks launched just before the while loop, after the start_thread call is complete, under the assumption that whatever start_thread might have done to that variable has been done. (An implementation could have some kind of markup on the thread creation function so that the compiler can infer the pointed-at memory has become volatile... but I doubt anybody actually does that. Much easier to let the code fuck up and blame the programmer ;))
vinkelhake 13y ago
To expand on what to3m said. The code was transformed into something like this: int launched = 0; start_thread(...); if (!launched) { while (true) {} } That's a perfectly legal optimization. The integer here isn't atomic and if another thread modifies the variable, that would be a data race and those have undefined behavior. Therefore, the compiler can assume that there is no race and move the comparison out of the loop. Changing it to be "std::atomic<int> launched;" fixes that problem.
- cube13 13y ago
  >That's a perfectly legal optimization. The integer here isn't atomic and if another thread modifies the variable, that would be a data race and those have undefined behavior. Therefore, the compiler can assume that there is no race and move the comparison out of the loop. In this case, the data race is fairly minimal, since we're talking about a 4 byte integer being used as a boolean flag. Operations like x = 1 for 4 byte ints are pretty much guaranteed to be atomic with x86, and worst case, you go through 1 more iteration of your loop than you would have otherwise(which your code should account for at any rate with locking for critical sections). Any change to the int should cause the while loop to exit. It should be a volatile(C pre C11) or atomic(C++11/C11) integer if you're passing it between threads, though.
- deleted 13y ago
  [deleted]

by nly 13y ago

to3m 13y ago

It presumably checks launched just before the while loop, after the start_thread call is complete, under the assumption that whatever start_thread might have done to that variable has been done. (An implementation could have some kind of markup on the thread creation function so that the compiler can infer the pointed-at memory has become volatile... but I doubt anybody actually does that. Much easier to let the code fuck up and blame the programmer ;))

vinkelhake 13y ago

To expand on what to3m said. The code was transformed into something like this: int launched = 0; start_thread(...); if (!launched) { while (true) {} } That's a perfectly legal optimization. The integer here isn't atomic and if another thread modifies the variable, that would be a data race and those have undefined behavior. Therefore, the compiler can assume that there is no race and move the comparison out of the loop. Changing it to be "std::atomic<int> launched;" fixes that problem.

cube13 13y ago
>That's a perfectly legal optimization. The integer here isn't atomic and if another thread modifies the variable, that would be a data race and those have undefined behavior. Therefore, the compiler can assume that there is no race and move the comparison out of the loop. In this case, the data race is fairly minimal, since we're talking about a 4 byte integer being used as a boolean flag. Operations like x = 1 for 4 byte ints are pretty much guaranteed to be atomic with x86, and worst case, you go through 1 more iteration of your loop than you would have otherwise(which your code should account for at any rate with locking for critical sections). Any change to the int should cause the while loop to exit. It should be a volatile(C pre C11) or atomic(C++11/C11) integer if you're passing it between threads, though.
deleted 13y ago
[deleted]