Why is the waitcnt getting caught in the 53 second loop?

SRLM · 2013-06-29 07:14

I'm converting the Eddie.spin file to C++, and this loop is problematic for me:

NextCNT := PROCESSRATE + cnt
repeat while (rxcheck := Term.rxcheck) < 0          ' Read a byte from the command UART
  LoopCount++
  if KeepAlive and LoopCount > constant(_clkfreq / PROCESSRATE)' Stop motors if no com in one second
    ifnot PDRunning
      outa[24..19]~~
    waitcnt(constant(_clkfreq / DIVISOR) + cnt)
    SetPower[LEFT_MOTOR]~
    SetPower[RIGHT_MOTOR]~
    StillCnt~
    Mode := POWER
  else
    waitcnt(NextCNT += PROCESSRATE)
InputBuffer[InputIndex++] := rxcheck

In Spin, PROCESSRATE is 5000 (CLKFREQ/16000). Spin2cpp generates the following code:

Nextcnt = (Processrate + CNT);
    Term.Str((int32_t)"Here outer loop!");
    while ((Rxcheck = Term.Rxcheck()) < 0) {
    	Term.Str((int32_t)"Here2!");
      (Loopcount++);
      if ((((uint8_t *)&dat[955])[0]) && (Loopcount > 0x3e80)) {
        if (!(((uint8_t *)&dat[954])[0])) {
          OUTA = ((OUTA & 0xfe07ffff) | 0x1f80000);
        }
        waitcnt((0x186a00 + CNT));
        ((int32_t *)&dat[408])[Left_motor] = 0;
        ((int32_t *)&dat[408])[Right_motor] = 0;
        ((uint8_t *)&dat[436])[0] = 0;
        ((uint8_t *)&dat[438])[0] = Power;
      } else {
        waitcnt((Nextcnt = (Nextcnt + Processrate)));
      }
    }
    ((uint8_t *)&dat[439])[((*(uint8_t *)&dat[949])++)] = Rxcheck;

In the C++ version I have to increase Processrate to about 75000 so that it doesn't hang in the waitcnt. I'm compiling with -mcmm and -Os. Is it really that much slower?

ersmith · 2013-06-29 12:02

The original Spin code looks a bit suspicious to me. Shouldn't Nextcnt be updated if the first branch in the IF is taken? That part of the IF will take significantly more than 5000 cycles to execute.

I see you've inserted some debug prints into the C++ version. Those will obviously take more than 5000 cycles to execute, so again Nextcnt is going to have to be updated after them.

SRLM · 2013-06-29 12:46

The source spin code seems to run fine. I'm suspicious too, but for a different reason: why have a "process rate" at all? This is the code that processes the incoming bytestream on the serial port, so I figure we'd want to go as fast as possible.

The debug points do slow down the code, but it exhibits the same locking behavior in C++ without them.

jazzed · 2013-07-01 07:18

Cody, if the function can be fcached it would be faster. I.E.

__attribute__((fcache)) void somefunction() {}

Functions should be small to use fcache.

In a related point ....

Looking at built-in waitcnt code generation, it looks like CNT is loaded before adding the offset. This is not optimal.

 144 00c2 F2000EA0         mov    r7, CNT
 145                  .LVL14
 146 00c6 371301           add    r7, #275
 147 00c9 F3000EF8         waitcnt    r7,#0

This would be better.

mov    r7, #275
add    r7, CNT
waitcnt    r7,#0

ersmith · 2013-07-01 12:02

jazzed wrote: »

In a related point ....

Looking at built-in waitcnt code generation, it looks like CNT is loaded before adding the offset. This is not optimal.

That's not a part of the waitcnt code generation, though, it's the code generation for the expression "CNT + PROCESSRATE" inside waitcnt(CNT+PROCESSRATE). waitcnt itself just takes a single argument, and the code generation for it has no way of knowing how that argument was obtained.

waitcnt2() takes two arguments, and is probably better for most purposes.

Eric

jazzed · 2013-07-01 14:23

ersmith wrote: »

That's not a part of the waitcnt code generation, though, it's the code generation for the expression "CNT + PROCESSRATE" inside waitcnt(CNT+PROCESSRATE). waitcnt itself just takes a single argument, and the code generation for it has no way of knowing how that argument was obtained.

waitcnt2() takes two arguments, and is probably better for most purposes.

Eric

FWIW: The dump was from another program that uses from waitcnt propeller.h which is a variant of waitcnt2.

Original statement: waitcnt(period+CNT); The order of the sum does not have any affect.

David Betz · 2013-07-01 19:06

jazzed wrote: »

FWIW: The dump was from another program that uses from waitcnt propeller.h which is a variant of waitcnt2.

Original statement: waitcnt(period+CNT); The order of the sum does not have any affect.

Addition is commutative and the GCC code generator knows that so it will reorder the evaluation of the operands however it thinks best. It has no way of knowing that CNT should be handled in any special way.

jazzed · 2013-07-02 10:51

The point is that builtin_waitcnt should be able to distinguish between CNT which is at a fixed address and some other variable and produce the right code.

David Betz · 2013-07-02 11:12

jazzed wrote: »

The point is that builtin_waitcnt should be able to distinguish between CNT which is at a fixed address and some other variable and produce the right code.

As Eric said, the expression "period+CNT" has already been evaluated before the builtin_waitcnt function is compiled. GCC doesn't know that it contained a reference to CNT. I guess you could make a new builtin function that does CNT relative waiting. That could make sure that CNT is read just before the waitcnt instruction is executed. Maybe something ugly like "builtin_waitcnt_relative". :-(

jazzed · 2013-07-02 12:04

I'm sorry. I forgot that CNT is implied in waitcnt rather than a parameter.

It's really too bad that we can't just do the right thing though because people historically have tripped over the order in which parameters are placed before waitcnt is executed.

Seems like in this case at least that the builtin waitcnt is not of much value to anyone wanting the smallest possible wait, and should be replaced with an appropriate in-line asm. Fortunately we have the power to do that.

David Betz · 2013-07-02 12:08

jazzed wrote: »

I'm sorry. I forgot that CNT is implied in waitcnt rather than a parameter.

It's really too bad that we can't just do the right thing though because people historically have tripped over the order in which parameters are placed before waitcnt is executed.

Seems like in this case at least that the builtin waitcnt is not of much value to anyone wanting the smallest possible wait, and should be replaced with an appropriate in-line asm. Fortunately we have the power to do that.

As I said, we could have a relative version of the waitcnt builtin. Also, I think the order of the arguments may be less of a concern for C code than it is for Spin code. You'll only really run into trouble if you're trying to push the envelope and write code that barely fits within the Propeller timing.

jazzed · 2013-07-02 12:19

David Betz wrote: »

As I said, we could have a relative version of the waitcnt builtin. Also, I think the order of the arguments may be less of a concern for C code than it is for Spin code. You'll only really run into trouble if you're trying to push the envelope and write code that barely fits within the Propeller timing.

As has recently happened to me. Of course this epiphany most likely didn't help solve the original question though.

David Betz · 2013-07-02 12:55

jazzed wrote: »

As has recently happened to me. Of course this epiphany most likely didn't help solve the original question though.

It seems to me that if you're really pushing the envelope and using timing that barely fits then you should probably be using PASM.

Why is the waitcnt getting caught in the 53 second loop?

Comments