6

I have a question regarding optimizations the compiler can potentially do.

The below code will speak for itself (this is an example):

typedef struct  test
{
  short i;
}               s_test;

int function1(char *bin)
{
  s_test foo;

  lock(gmutex);
  foo.i = *(int*)bin * 8;
  unlock(gmutex);

  sleep(5);
  //
  // Here anything can happen to *bin in another thread
  // an inline example here could be: *(volatile int *)bin = 42;
  //

  int b = foo.i + sizeof(char*);

  return (b > 1000);
}

Could the compiler ever replace the last lines with

return ((*(int*)bin * 8 + sizeof(char*)) > 1000);

It did not seem to be the case using -O2 or -O3 with gcc 4.4 but could it be the case with other compilers and with other compilation flags?

undur_gongor
  • 15,657
  • 5
  • 63
  • 75
Dpp
  • 1,926
  • 1
  • 18
  • 26
  • 5
    Replacing a variable by an expression isn't exactly an optimization, is it? – user207421 Jun 05 '12 at 07:30
  • 1
    For some issues wrt compiler optimization and thread-safety, see http://stackoverflow.com/questions/2001913/c0x-memory-model-and-speculative-loads-stores and in particular the documents linked therein. – janneb Jun 05 '12 at 07:50
  • 1
    Changes to *bin can also happen during your mutex unless you protect every access to _ALL_ memory objects *bin can point to with the gmutex. The only thing which is protected by your mutex is the local variable foo which is maybe not what you wanted. – slartibartfast Jun 05 '12 at 08:15

3 Answers3

5

I don't think compiler will do such kind of optimization.

unlock(gmutex)

this is function, the compiler can't assume that value pointed by bin will be changed in unlock function or not.

for example, maybe bin comes from a globe. So optimization for bin can't cross the function call.

RolandXu
  • 3,566
  • 2
  • 17
  • 23
  • 1
    The compiler certainly can't do this optimization. Otherwise there would be no safe way to implement a locking function. – ugoren Jun 05 '12 at 07:40
  • 2
    @ugoren, locking and thread model only come with C11. Before all bets are off for relying on the C standard itself concerning any motivation with this respect. So thread programming with C before C11 needs special care. In particular aliasing as is done in the example is not a good idea. Here it is probably ok since `bin` is `char*` but my guess would be that Dpp doesn't control this aspect of C very well :) – Jens Gustedt Jun 05 '12 at 07:51
  • Unless you got the "assume no aliasing across functions" flag set (and actually have a function that can change a local variable). Fortunately, I don't see this option documented any more on MSDN. – selbie Jun 05 '12 at 07:55
  • 1
    @JensGustedt, Locking and the thread model with C were possible and heavily used long before C11. An infrastructure developer had to know the platform details, but once he wrote the locking functions you could just call them, and trust that they would work. This would not be possible if the compiler was allowed to reorder reads around function calls. Aliasing can be tricky, but not only with multi-threading, and doesn't seem relevant in this case. – ugoren Jun 05 '12 at 08:01
  • @ugoren, I said nothing like that. I just said that you can't deduce things the way you did. You'd have to look into guarantees that a particular thread platform gives you. An yes aliasing has to do with it. Type punning leads easily leads to undefined behavior, so all bets are off. – Jens Gustedt Jun 05 '12 at 08:44
  • @JensGustedt, Type punning here may be a problem, but is unrelated to threading. In this question, the worry was that the compiler would rearrange code around a function call. This no compiler can do (and if anyone wrote such a compiler, locking wouldn't be possible), so in this respect, the code is safe. – ugoren Jun 05 '12 at 10:52
  • @ugoren What makes you believe that implementing locking is possible in any compiler using only C/C++ std features (that predates std threads)? And if non std features are used anyway, why not simply use an attribute that means "function has unlock semantics"? – curiousguy Apr 29 '19 at 20:52
4

Your example is unnecessarily complicated because you are reading bin through a different type than it is declared. Aliasing rules are quite complicated, char is even special, I wouldn't comment on that.

Supposing your declaration would be int* bin (and so you wouldn't have to cast the pointer) a compiler would not have the right to reorder statements across function calls, they form so-called sequence points. The value of *bin before and after the call to unlock could be different, so it has to load the value after the call.

Edit: as noted by slartibartfast, for this argument it is essential that unlock is (or contains) a function call and isn't just a macro that resolves to a sequence of operations by the compiler.

Jens Gustedt
  • 76,821
  • 6
  • 102
  • 177
  • Sorry to intercept again: even if unlock() is just a statement it would contain a ";" which is a sequence point like a function call you are trying to pull in front of the curtain - it's just that this won't help in that case. To get a feeling for the intricacies of multithreaded C together with the standards up to C99 please have a look at comp.lang.c and comp.std.c archives. – slartibartfast Jun 05 '12 at 09:22
  • If `unlock` is "just a macro that resolves to a sequence of operations by the compiler", how can it unblock another blocked thread? Which operation known to the compiler not to modify global variables can act on other threads? – curiousguy Apr 29 '19 at 21:14
1

As I said in the direct reply: your mutex isn't protecting anything IMHO. The char array which *bin points into can get modified during the int-access so depending on your machine you won't even get a consistent view on the memory you wanted to access. Back to your question: a compiler will not transform the source code to the sequence which you envisioned BUT it may very well produce machine language which in effect will behave the way your source code does. If it is able to inspect the functions lock, unlock and sleep (which seem to be macros anyway) and can deduce that there is no side effect to the involved memory locations AND there is no implementation defined meaning to calls to e.g. sleep() which would render temporary ("cached" although the standard doesn't use this term) values invalid then it is entitled to produce an instruction sequence like the one you gave. C (up to C99) is inherently single-threaded and the compiler can employ any optimization strategy it wants as long as the code behaves "as if" it would run on the ideal hypothetical C machine. The sequence points which Jens mentioned don't affect the correctness under single threaded conditions: the compiler could hold foo in a register during the whole function or it could even alias foo.i with the memory location pointed to by *bin, so this code is inherently dangerous (alhtough I think it will not exhibit this behaviour on most compilers).

slartibartfast
  • 641
  • 4
  • 12
  • You are right if you are assuming that the `unlock` function may not be a function call but a macro that expands to a sequence of operations that are known to the compiler. I was assuming the other case, that this is effectively a function call. – Jens Gustedt Jun 05 '12 at 08:50
  • 2
    Even then this will not save you. If the compiler can look into the function it may very well optimize across it. Having foo as an automatic is a somewhat unlucky premise for this example. – slartibartfast Jun 05 '12 at 08:55
  • There's no problem if `lock` and `unlock` are not buggy. Implementing them in a macro, or a function in the same scope, that doesn't somehow prevent reordering is one of many subtle ways to write buggy locking primitives. Also, the lock protects accesses to `*bin` if it's held whenever this memory is accessed. Locking in just one place is worthless. – ugoren Jun 05 '12 at 10:56