Timing for a compound if( ___ && ___)

The time saving of moving things in an if() had the opposite of what I expected.

I would have thought that placing the 'flag test' at the front would have given a shorter time but it did not.

  if (millis() - lastMillis >= 500 && flag == false)
  {
    //this gives 1.25us
  }

  if (flag == false && millis() - lastMillis >= 500)
  {
    //this gave 1.563us
  }

300ns difference

My guess is the compiler does something I had not considered.

Learn something new every day. :o

That's unexpected indeed!

Even the Compiler Explorer has no answers ...

I don't think the compiler is allowed to swap the conditions, because if flag == true in the second case, millis should not be called (it could have side effects).

Could you post the full code you used to test it?

Pieter

As requested:

//Timing measurement done with a logic analyzer looking at D13

#define pulse13 PINB = 0x20; PINB = 0x20

boolean flag = false;

unsigned long lastMillis;

void setup()
{
  pinMode(13, OUTPUT);
}

void loop()
{
  pulse13;
  if (millis() - lastMillis >= 500 && flag == false)
  {
    //this gave 1.25us
  }
  pulse13;

  pulse13;
  if (flag == false && millis() - lastMillis >= 500)
  {
    //this gave 1.563us
  }
  pulse13;

  delayMicroseconds(100);

} //END of loop()

  if (flag == false && millis() - lastMillis >= 500)
I would have thought that placing the 'flag test' at the front would have given a shorter time but it did not.

It depends. If the "flag test" evaluates to TRUE, then the code has to do both tests regardless, so the timing should be the same. If the first test evaluates to FALSE, the second test doesn't need to be evaluated. So having the boolean check first only speeds things up when the result is FALSE.
In your "full example", flag is a compile-time constant, so it's check is entirely eliminated from the executable code, and the if clauses are empty, so most of the conditional code is removed as well, leaving only the reads/writes of volatiles that are forced upon the compiler...

You really can't analyze code execution to this degree without looking at the asm...
All you get is:

void loop() {
 228:   c0 e2           ldi     r28, 0x20       ; (bit13 bit)
 22a:   c3 b9           out     0x03, r28       ; toggle
 22c:   c3 b9           out     0x03, r28       ; toggle

 22e:   8f b7           in      r24, 0x3f       ; read millis atomically!
 230:   f8 94           cli
 232:   40 91 05 01     lds     r20, 0x0105     ; 0x800105 <timer0_millis>
 236:   50 91 06 01     lds     r21, 0x0106     ; 0x800106 <timer0_millis+0x1>
 23a:   60 91 07 01     lds     r22, 0x0107     ; 0x800107 <timer0_millis+0x2>
 23e:   70 91 08 01     lds     r23, 0x0108     ; 0x800108 <timer0_millis+0x3>
 242:   8f bf           out     0x3f, r24       ; fix SREG (~11 cycles for millis()?)

 244:   c3 b9           out     0x03, r28       ; toggle
 246:   c3 b9           out     0x03, r28       ; toggle
;; (repeat)

They two sequences the same on my scope - about 750ns between blips (~11 cycles for reading timer0_millis - seems about right.)
(Since this doesn't agree with what you were seeing, perhaps... it's not quite the same code?)

  if (millis() - lastMillis >= 500 && flag == false)
  {
    //this gave 1.25us
  }

The body is empty, so the value of the condition becomes irrelevant and the comparisons are optimized away. The only thing that cannot be removed is calling millis().

Adding someting (like PINB = 0x10) to the body should make this test more meaninful.

westfw:
It depends. If the "flag test" evaluates to TRUE, then the code has to do both tests regardless, so the timing should be the same. If the first test evaluates to FALSE, the second test doesn't need to be evaluated. So having the boolean check first only speeds things up when the result is FALSE.
In your "full example", flag is a compile-time constant, so it's check is entirely eliminated from the executable code, and the if clauses are empty, so most of the conditional code is removed as well, leaving only the reads/writes of volatiles that are forced upon the compiler...

. . .

They two sequences the same on my scope - about 750ns between blips (~11 cycles for reading timer0_millis - seems about right.)
(Since this doesn't agree with what you were seeing, perhaps... it's not quite the same code?)

Once again you have restored my faith!

~800ns less if I use true and move the Flag test to the front. :slight_smile:

//Timing measurement done with a logic analyzer looking at D13

#define pulse13 PINB = 0x20; PINB = 0x20

boolean flag = false;

unsigned long lastMillis;

void setup()
{
  pinMode(13, OUTPUT);
  pinMode(12, OUTPUT);
}

void loop()
{
  pulse13;
  if (millis() - lastMillis >= 500 && flag == true)
  {
      //This gave 1.24us
  }
  pulse13;

  pulse13;
  if (flag == true && millis() - lastMillis >= 500)
  {
      //This gave .44us
  }
  pulse13;

  delayMicroseconds(100);

} //END of loop()

The compiler is smart enough to notice that 'flag' is initialized and never modified so it can be treated as a constant. One way to avoid having the compiler optimize away code that you want to time is to declare variables 'volatile'.

That’s interesting John.
I try to see what that does, after snow shoveling :frowning: