11

The following testing code does correctly in VS either with debug or release, and also in GCC. It also does correctly for ICC with debug, but not when optimization enabled (-O2). 

#include <cstdio>

class tClassA{
public:
  int m_first, m_last;

  tClassA() : m_first(0), m_last(0) {}
  ~tClassA() {}

  bool isEmpty() const {return (m_first == m_last);}
  void updateFirst() {m_first = m_first + 1;}
  void updateLast() {m_last = m_last + 1;}
  void doSomething() {printf("should not reach here\r\n");}
};

int main() {
  tClassA q;
  while(true) {
    while(q.isEmpty()) ;
    q.doSomething();
  }
  return 1;
}

It is supposed to stop at while(q.isEmpty()). When -O2 enabled under ICC (release), however, it starts to "doSomething" infinitely.

Since this is single-threaded program and isEmpty() should be evaluated as true, I can find no reason the ICC should behave in this way? Do I miss anything?

Ciro Santilli OurBigBook.com
  • 347,512
  • 102
  • 1,199
  • 985
Samuel
  • 111
  • 3
  • does it help if m_first and m_last are declared as 'volatile'? I don't have access to ICC. – Chubsdad Aug 20 '10 at 02:54
  • Another wild thought: has it got to do with the fact that %s is not specified with printf. printf("%s", "should not reach here\r\n");? – Chubsdad Aug 20 '10 at 02:58
  • 5
    Related: [Are compilers allowed to eliminate infinite loops?](http://stackoverflow.com/questions/2178115/are-compilers-allowed-to-eliminate-infinite-loops) – James McNellis Aug 20 '10 at 03:09
  • It would be helpful for you to indicate which version of ICC you are testing and on what platform. – James McNellis Aug 20 '10 at 03:14
  • @Samuel: Would like to know for my reference if making the members 'volatile' helped. That should suppress any optimization flag effects – Chubsdad Aug 20 '10 at 05:23
  • 1. If m_first and m_last are declared as "volatile", there is no issue; if isEmpty() is declared as "volatile", there is no issue neither. 2. I used ICC 10 with VS – Samuel Aug 20 '10 at 07:54
  • As for printf..., it doesn't matter, you can do anything in this function, e.g., {m_first ++; m_last ++; ...} – Samuel Aug 20 '10 at 08:10

7 Answers7

9

Because the while (q.isEmpty()) ; loop contains no statements that can ever cause an externally-visible side-effect, the entire loop is being optimised out of existence. It's the same reason that:

for (int i = 0; i < 10; i++)
    ;

could be optimised out of existence, as long as i was not volatile (stores to volatile objects are part of the "externally visible" effects of a program).

In the C language, it is actually an outstanding bone of contention as to whether an infinite loop is allowed to be optimised away in this manner (I do not know what the situation is with C++). As far as I know, a consensus has never been reached on this issue - smart and knowledgeable people have taken both sides.

caf
  • 233,326
  • 40
  • 323
  • 462
  • This could be the reason. The following code works correctly: static int A=0; while(q.isEmpty()) {A = A+ 1;} But not the following: static int A=1; while(q.isEmpty()) {A;} – Samuel Aug 20 '10 at 08:03
  • 1
    Interesting. An infinite loop doing nothing - nice optimization opportunity (and it is understandable why someone may disagree). – Suma Aug 20 '10 at 08:11
  • One more thing: if while(true) is used for while(q.isEmpty()), the code works correctly. – Samuel Aug 20 '10 at 08:15
  • Suma: As far as the standard goes, it is correct behavior, since it does not have any visible side-effects, it can't possibly change the behavior of the program, right? (Wrong - an infinite loop DOES change the program, but since detecting if the loop is infinite or not, in the general case, is basically the halting problem, it makes the decision to always remove such loops. If this is undesirable, then it is up to you, the programmer, to tell the compiler that it does in fact have side-effects, eg, by reading or writing a volatile varibale or some compiler-specific flags should they exist) –  Jan 28 '11 at 04:49
  • 2
    @Samuel: Note that in your first example, `A` overflows eventually, producing Undefined Behavior. Since the optimizer may assume that UB does not happen, it can deduce that `q.isEmpty()` must be false at the start, and the entire loop can be eliminated. – MSalters Mar 26 '13 at 08:06
2

It sure sounds like a bug. Here's a (pretty wild) guess about what reasoning might have lead to it...

After inlining, it sees:

while (q.m_first == q.m_last) /* do nothing */ ;
do_something();

and any sequence of do nothing repeatedly ; do something can be translated to simply "do something". This falls down if the repeated part is endless (as in this case). But perhaps they don't test their compiling on examples that intentionally have endless looping ;-).

Edmund
  • 10,533
  • 3
  • 39
  • 57
  • Detecting intentional endless looping is basically the halting problem, so instead they assume that, since it has no visible side-effects, it does nothing and therefore is useless to the program and may be removed. Obviously, whether or not it completes IS a visible sde-effect for us, but not for the C/C++ abstract machine defined by the standard. If you want the loop, then you must tell the compiler that it does, indeed, affect the program by, for exmaple, making one of the read variables volatile –  Jan 28 '11 at 04:47
1

Any chance the actual code that you built and ran was missing the semicolon after while(q.isEmpty()) ? That would certainly result in the next line being called infinitely.

TheUndeadFish
  • 8,058
  • 1
  • 23
  • 17
  • 1. there is semicolon 2. the same result with while(q.isEmpty()) {}, or while(q.isEmpty()) {true;} – Samuel Aug 20 '10 at 08:06
  • To whoever: Was a down-vote really necessary? I've seen a number of cases on SO where the posted code was not the exact code being used and where actual problem had been left out or fixed in the translation. I just wanted to help make sure this wasn't the result of a simple but easy to overlook mistake. (After all, haven't we all occasionally wasted time on a bug due to something like a mistaken semicolon?) – TheUndeadFish Aug 21 '10 at 00:43
1

As a slight aside, this version of icc does what you want. That is, it never calls doSomething().

[9:41am][wlynch@computer /tmp] icc --version
icc (ICC) 11.0 20081105
Bill Lynch
  • 80,138
  • 16
  • 128
  • 173
1

The C++ standard allows loops without side-effects to be removed, even if they don't terminate:

It is generally felt that it is important to allow the transformation of potentially non-terminating loops (e.g. by merging two loops that iterate over the same potentially infinite set, or by eliminating a side-effect-free loop), even when that may not otherwise be justified in the case in which the first loop never terminates. http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2429.htm

See the discussion here: http://blog.regehr.org/archives/161

Charles
  • 11,269
  • 13
  • 67
  • 105
0

Your best bet is to take the resulting binary step into it and dis-assemble the main function and see what assembly was generated. Not saying you will be able to see a bug, but you can see if something was optimized out.

linuxuser27
  • 7,183
  • 1
  • 26
  • 22
-3

I think it may have been your version of gcc. I compiled your prog under 4.4.2 and it worked exactly as it should have.

BT.
  • 229
  • 1
  • 9