Stupidity in C++ processor

[Collin Kidder]

Arduino IDE 1.6.9 (or several past versions) and Due support files
1.6.8 (or 1.6.7 or 1.6.6, maybe more). I posted this question to the
users forum as well but I think it's kind of out of place and more
investigation shows this to almost certainly be a core bug in Arduino.

It seems that the preprocessing step somewhere along the way is being
really stupid.

I have the following code:

uint32_t upperDuty = MAX_PWM_DUTY - PWM_BUFFER;
uint32_t dutyA = a, dutyB = b, dutyC = c;

if (dutyA < PWM_BUFFER) dutyA = 0;
if (dutyB < PWM_BUFFER) dutyB = 0;
if (dutyC < PWM_BUFFER) dutyC = 0;

if (dutyA > upperDuty) dutyA = MAX_PWM_DUTY;
if (dutyB > upperDuty) dutyB = MAX_PWM_DUTY;
if (dutyC > upperDuty) dutyC = MAX_PWM_DUTY;

MAX_PWM_DUTY is 1000, PWM_BUFFER is 45. I know this for sure. That
should make upperDuty be 955. Instead, reading the assembly dump I see
that it really has a value of 1031. This can be seen here (value off
by one because of how the compiler generated code)

80d20: 2e2c cmp r6, #44 ; 0x2c
80d22: bf98 it ls
80d24: 2600 movls r6, #0
80d26: 2d2c cmp r5, #44 ; 0x2c
80d28: bf98 it ls
80d2a: 2500 movls r5, #0
80d2c: 2c2c cmp r4, #44 ; 0x2c
80d2e: bf98 it ls
80d30: 2400 movls r4, #0
80d32: f5b6 6f81 cmp.w r6, #1032 ; 0x408
80d36: bf28 it cs
80d38: f44f 767a movcs.w r6, #1000 ; 0x3e8
80d3c: f5b5 6f81 cmp.w r5, #1032 ; 0x408
80d40: bf28 it cs
80d42: f44f 757a movcs.w r5, #1000 ; 0x3e8
80d46: b2b2 uxth r2, r6
80d48: f5b4 6f81 cmp.w r4, #1032 ; 0x408
80d4c: 480d ldr r0, [pc, #52] ; (80d84 <_Z9updatePWMjjj+0xac>)
80d4e: f04f 0100 mov.w r1, #0
80d52: bf28 it cs
80d54: f44f 747a movcs.w r4, #1000 ; 0x3e8

This makes it pretty plain that PWM_BUFFER really was 45 and
MAX_PWM_DUTY really was 1000 because you can see those constants as
well. But, magically the computed constant is wrong.

Here's another one:
#define PWM_BUFFER ((PWM_BUFFER_NANOS * 500ul) / PWM_PICO_PER_TICK) +
PWM_DEADTIME + PWM_DEADTIME

That line compiles fine and properly turns into 45 like it should.
But, changing it to:
#define PWM_BUFFER ((PWM_BUFFER_NANOS * 500ul) / PWM_PICO_PER_TICK) +
(PWM_DEADTIME * 2)

Makes it not actually multiply by 2 so instead the value is something like 27.

Basically, the preprocessor is seriously borked at least in recent
versions of the IDE and it produces incorrect numeric constants
seemingly at random. This is a serious problem.

[Collin Kidder]

I've found that it only happens with complicated defines. If I set
each of those defines to a literal it compiles properly and reports
the proper value. But, using a bunch of constants with math operations
causes the value of each define to come out properly but doing math on
the defines later on breaks.

Here's a more or less minimal example:

#define PWM_FREQ 7000
#define MAX_PWM_DUTY 1000
#define PWM_CLOCK (PWM_FREQ * MAX_PWM_DUTY * 2ul)
#define PWM_PICO_PER_TICK (1000000000ul / (PWM_CLOCK / 1000ul))
#define PWM_TARGET_DEADTIME 1300
#define PWM_DEADTIME ((PWM_TARGET_DEADTIME * 1000ul) / PWM_PICO_PER_TICK) + 1
#define PWM_BUFFER_NANOS 1000

#define PWM_BUFFER ((PWM_BUFFER_NANOS * 500ul) /
PWM_PICO_PER_TICK) + PWM_DEADTIME + PWM_DEADTIME

void setup() {
}

void loop() {
uint32_t temp = MAX_PWM_DUTY - PWM_BUFFER;
SerialUSB.println(MAX_PWM_DUTY);
SerialUSB.println(PWM_BUFFER);
SerialUSB.println(temp);
SerialUSB.println();
delay(1000);
}

Changing the MAX_PWM_DUTY and PWM_BUFFER defines to just a number
makes it work. Leaving them as-is with math causes them to have the
proper values but temp to be wrong. It might be relevant to mention
two more things:

1. PWM_BUFFER will not have the proper value if that line ends in +
(PWM_DEADTIME * 2) instead of adding the value twice as seen above
2. Even the 1.5.4 version of the IDE seems to have this same problem.
So, the issue is old.

[Álvaro Lopes]

Try ... encosing all your defines with parenthesis

( (PWM_TARGET_DEADTIME * 1000ul) / PWM_PICO_PER_TICK) + 1 )

Looks like operation priority is messing your calculations. It's not pre-compiler fault, it's probably yours.

PWM_DEADTIME and PWM_BUFFER are nor properly protected.

Alvie

[Collin Kidder]

Yep, that'll do it. I forgot to enclose the defines and the compiler
was evaluating the whole string of math operations in one go. I
couldn't figure out how the defines could have the proper value but
math operations involving them didn't. That makes perfect sense and it
does compile and evaluate properly now. Thanks for setting me
straight.

[Andrew Kroll]

Any time you do operations in a macro, you need to protect it with () around the formula.

Glad you learned that. I learned this long ago -- perhaps 30 years ago.

It is more of a common pitfall than you may think.

--
You received this message because you are subscribed to the Google Groups "Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to developers+...@arduino.cc.

--

Visit my github for awesome Arduino code @ https://github.com/xxxajk

[Álvaro Lopes]

Just to close this thread, an additional note:

If you use parameterized macros, also make sure (unless in very specific situations) to also protect your arguments in parenthesis:

So, instead of:

#define TWICE(x) (2 * x)

Use:
#define TWICE(x) (2 * (x) )

Tip: expand TWICE( 1 + y )...

Alvie

On 27/05/16 20:40, Andrew Kroll wrote:
> Any time you do operations in a macro, you need to protect it with () around the formula.
> Glad you learned that. I learned this long ago -- perhaps 30 years ago.
> It is more of a common pitfall than you may think.
>
>
>
> On Fri, May 27, 2016 at 2:53 PM, Collin Kidder <col...@kkmfg.com <mailto:col...@kkmfg.com>> wrote:
>
> Yep, that'll do it. I forgot to enclose the defines and the compiler
> was evaluating the whole string of math operations in one go. I
> couldn't figure out how the defines could have the proper value but
> math operations involving them didn't. That makes perfect sense and it
> does compile and evaluate properly now. Thanks for setting me
> straight.
>

> To unsubscribe from this group and stop receiving emails from it, send an email to developers+...@arduino.cc <mailto:developers+...@arduino.cc>.

[Dennis Lee Bieber]

On Fri, 27 May 2016 14:31:13 -0400, Collin Kidder
<col...@kkmfg.com> declaimed the following:

>I've found that it only happens with complicated defines. If I set
>each of those defines to a literal it compiles properly and reports
>the proper value. But, using a bunch of constants with math operations
>causes the value of each define to come out properly but doing math on
>the defines later on breaks.
>
>Here's a more or less minimal example:
>
>#define PWM_FREQ 7000
>#define MAX_PWM_DUTY 1000
>#define PWM_CLOCK (PWM_FREQ * MAX_PWM_DUTY * 2ul)
>#define PWM_PICO_PER_TICK (1000000000ul / (PWM_CLOCK / 1000ul))
>#define PWM_TARGET_DEADTIME 1300
>#define PWM_DEADTIME ((PWM_TARGET_DEADTIME * 1000ul) / PWM_PICO_PER_TICK) + 1
>#define PWM_BUFFER_NANOS 1000
>#define PWM_BUFFER ((PWM_BUFFER_NANOS * 500ul) / PWM_PICO_PER_TICK) + PWM_DEADTIME + PWM_DEADTIME
>
>void setup() {
>}
>
>void loop() {
> uint32_t temp = MAX_PWM_DUTY - PWM_BUFFER;

You have take into account that C #define is a compile time string
substitution. That makes the above line

uint32_t temp = 1000 - (((1000 * 500ul) / (1000000000ul / ( (7000 * 1000 *
2ul) / 1000ul))) + ((1300 * 1000ul) / (1000000000ul / ( (7000 * 1000 * 2ul)
/ 1000ul))) + 1 + ((1300 * 1000ul) / (1000000000ul / ( (7000 * 1000 * 2ul)
/ 1000ul))) + 1


There is nothing wrong with the compiler preprocessor...

Wrap all your define values with () and I suspect it will work.
--
Wulfraed Dennis Lee Bieber AF6VN
wlf...@ix.netcom.com HTTP://wlfraed.home.netcom.com/