2


My question is not about double check locking but similar.
In the following code, is it possible for the compiler to optimize the inner if statement out?


    void MyClass:foo() 
    {
       if(x)   //member data of MyClass defined as bool
       {
           loc.lock(); //mutex.  Unlocks when out of scope
           if(x) //can the compiler optimize this out?
           {
             fire1();//doSomthingNotAffectingX
           }
        }
    }

X gets set via another thread in the same translation unit.


    void MyClass::unset()
    {
        loc.lock();
        x=false;
        fire2();
    }

The idea is to guarantee that if fire2 was called, fire1 can't be called

adamt
  • 21
  • 2
  • 1
    There's no guarantee that bool is atomic, you need to use std::atomic from C++11 or an older platform specific type to even get close to what you want. – Flexo Oct 31 '15 at 20:43
  • 1
    Why would I need atomic if I am inside a mutex lock? – adamt Nov 04 '15 at 03:35
  • 1
    Your first `if(x)` is outside the lock – Flexo Nov 04 '15 at 07:17
  • @Flexo, if `x` is never set to true (except at creation), then it does not matters that the first if is outside. The first `if` is just an optimization to avoid locking. A similar pattern is often used for singletons. – Phil1970 Aug 15 '18 at 00:19
  • @Flexo If a variable is protected by a `mutex` you don't need to make it `atomic` too. Read [std::mutex](https://en.cppreference.com/w/cpp/thread/mutex) documentation. – Phil1970 Aug 15 '18 at 00:27
  • @Phil1970 if you want to write code that's always correct everywhere you need to make sure that one of the following statements is true: a) all read/write operations are done with a lock held or b) the type is atomic. In the above code neither of those statements is true, then all bets are off. https://en.cppreference.com/w/cpp/atomic/memory_order is a better reference than the mutex one for this. (There's also no reason to suppose that the mutex in the question is the C++11 one either) – Flexo Aug 15 '18 at 00:45
  • @Flexo See my updated answer. In the general case, the above code would be incorrect and to be on the safe side, best to avoid that kind of code. – Phil1970 Aug 15 '18 at 02:46
  • @adamt Since your lock syntax is not the one with std::mutex, std::lock_guard, you should be more precise as if `lock` is arbitrary (inline) code, the compiler might optimize the second check away. Also, does `x` can only goes from `true` to `false` as otherwise, the program is obviously wrong. – Phil1970 Aug 15 '18 at 02:52

2 Answers2

0

In my answer, I assume you are using std::mutex and not your own custom class!

Essentially, the compiler cannot optimize away the second if.

Well, if the compiler could somehow determine that x cannot be changed (legally), it might remove the check but clearly, this is not the case here. A compiler can only do optimization if the resulting program works AS-IF.

In C++ 11 and later, a mutex is a barrier. So if a variable is properly protected by a mutex, you would read the expected value. However, it you forgot to put some mutex at appropriate location, then the behavior will be undefined.

Here, as your first check is a read, you might have a problem, if x can again become true one after being set once to false.

Memory model

C++11 introduced a standardized memory model. What does it mean? And how is it going to affect C++ programming?

I highly recommend reading the book C++ Concurrency in Action, Practical Multithreading by Anthony Williams. It would help a lot understanding modern C++ multithreading.

In your code, do you need to protect fire1 and fire2 by the mutex? Calling a function can be dangerous if that function also wait for a mutex as it can cause deadlocks. Also, the lock might be effective longer than required. If you only need to ensure that fire1is not called if fire2 is called, then a std::atomic<bool> would be enough…

-- EDIT --

Finally, there is a good example in the documentation: std::mutex

-- EDIT #2 --

As pointed out in a comment, my answer is not fully valid for general cases. I do think that the above code would works correctly with a bool provide that it can only change from true to false.

So I have done a search on the web and found those articles:

Phil1970
  • 2,605
  • 2
  • 14
  • 15
-1

If the compiler doesn't know someone other than loc.lock() might change x, and it knows loc.lock() won't change x for sure, then it can assume x won't change between the ifs. In this case, it is likely to store x in a register, or omit the if altogether.

If you want to avoid such unsafe optimizations, you have to let the compiler know. On C++11, use std::atomic<bool>. On earlier versions, make it volatile bool. Do neither, and the compiler might break your code.

Eran
  • 21,632
  • 6
  • 56
  • 89
  • Doesn't the lock is a higher form of a memory barrier? If what you said is right, doesn't that mean all variable should be volatile in multithreaded programming? Consider this
    if(flag) {if(foo()) where foo(
    – adamt Nov 04 '15 at 03:38
  • Timed out. Sorry. Trying again. isn't the lock a higher form of a memory barrier? What is the point of using a mutex then? If what you said is right, doesn't that mean all variable should be volatile in multithreaded programming, due the fact that some inline function could end up expanding to nested if like the one above. Yet not too obvious to the programmer. – adamt Nov 04 '15 at 03:45
  • How would the compiler know the lock is a barrier? If you use dedicated language constructs (C++11) it can; if you use dedicated compiler extensions it can; and otherwise if the compiler doesn't know for sure it will probably be conservative and avoid the optimization. But if your lock is a simple spin lock, the compiler has no reason to consider it a memory barrier - just a simple loop (which might also be optimized). As for `volatile`, it a poor man's solution, but if that's all you have, all variables that are shared must be `volatile` (not solving all issues - lookup volatile). – Eran Nov 04 '15 at 08:19
  • @eran If you use dedicated language construct, then the compiler know somehow… And a compiler probably also know what the OS provide. So in practice, the only case where you would need to be really careful would be if you write your own lock. – Phil1970 Aug 15 '18 at 00:16
  • @eran Read documentation [std::mutex](https://en.cppreference.com/w/cpp/thread/mutex) – Phil1970 Aug 15 '18 at 00:25