Resolved: Special case conditional expression bug

[Anobium]

During a recent commercial project a serious bug was found when the compiler was generating the ASM for a conditional expression - the compiler was not handling Word values correctly. The result of the conditional expression was incorrect when the condition was adding a Word to a Byte variable.

The errant code example. match is a Bit type, the array elements are Word type. If the value of element 1 is 257 and element 3 is 260 then the match should be 0, when element 1 is 258 then match should be 1 - it was not.

'The result of the conditional expression is either 0 or 1
match =  times_array(1) + 2 >= times_array(3)

A full test program is attached (and is not included in the demos).

The fix was a long process to determine the root cause but is came down to the determination of the type of temporary variable required to hold the calculation times_array(1) + 2. The fix examines the caculation operation (CALCOPS) and determines the a SysTemp variable is needed;type of variables; it is a conditional expression; it is a maths operation - if all the cases are true then determine the correct variable type required.

To delivery the project, using the existing compiler, we simply used an IF-THEN statement but this took hours to determine.

'The result of the IF-THEN is either 0 or 1
If times_array(1) + 2 >= times_array(3) Then 
    match = 1 
Else 
    match = 0
End If

A simpler workaround is. Note the braces around the maths operation. The braces force the compiler to complete the maths operations into a correctly sized SysTemp variable.

'The result of the conditional expression is either 0 or 1
match =  ( times_array(1) + 2 ) >= times_array(3)

---

My belief is 1) the compiler assumes byte operations and this is issue was left over from the addition of non-Byte type variables and, 2) that this type of conditional expression was not possible to be created in GCGB therefore it was never tested.

---

I have tested on PIC and AVR using an LCD to show results of the conditional expression. See the attachment of the test program.

---

Anyway, all resolved in build 1113. Took two full days to find the root cause, fix and test the compiler.

As this is major change to the compiler the ASM will show when this specific condition is resolved. The following ASM will be created.

;COMPILER INFORMATION: CONDITIONAL EXPRESSION IN ASM ABOVE IS USING VARIABLE WORD TYPE. FIX 1113

Enjoy the fix...

[Anobium]

For AVR testing I used OshenSoft AVR Simulator. Very good simulator for those who have not tried it.

Showing the results of the bugcode.gcb program.

See the OshenSoft website.

[stan cartwright]

I have never used an expression like 'The result of the conditional expression is either 0 or 1
match = times_array(1) + 2 >= times_array(3)

I would maybe use
'The result of the IF-THEN is either 0 or 1
If (times_array(1) + 2) >= times_array(3) Then
match = 1
Else
match = 0
End If

This is not digressing but relevant for me.
Consider dim backgnd (256) as long.
is that an array of 256 long elements or it it a long array of bytes?
It was for a copy screen area routine

sub getbgnd
  ptr=1
  for hs=sprite_y to (sprite_y+sprite_height)
   for ws=sprite_x to (sprite_x+sprite_width)
      backgnd(ptr) = ReadPixel_ILI9341(ws,hs) and 0xfffff
      ptr++
   next ws
  next hs
end sub

[Anobium]

Your case is valid and works. I too would have used your approach. The case we found was very specific but it needed to be fixed.

:-)

[stan cartwright]

Cheers.I haven't tested it yet.
Your 'The result of the conditional expression is either 0 or 1
match = ( times_array(1) + 2 ) >= times_array(3)
The braces/brackets I always add to break the expression into order or what I guess the compiler does. It's best not to assume things. Do the braces use memory?

[Anobium]

The braces aid the compiler. The contents within a set of braces will be evaluated probably to a SysTemp var. The best thing.. add the braces.

[stan cartwright]

Last thought, I was taught mult,div.plus,minus. Stuff in braces done first. Does that apply to gcb? It that what the compiler expects?

[Anobium]

Hugh has done a good job on precedence. This is the way it is.

"*/%", then, "+-" then, "=~<>{}", then, "&|!#"

You will have to decode but } is equal or greater than.

:-)

[stan cartwright]

Hmmmmm... Ta :)
I tried
dim var1 (10) as byte
dim var2 (10) as word
dim var3 (10) as long
var1 (1)=100:var2 (2)=1000:var3 (3)=100000

glcdprint (0,0,str(var1(1)+var2(2)+var3(3)))
shows 35564 on glcd

[Anobium]

@Stan. The following code show what I call 'give the compiler a chance'..

This was tested on a 16F. Mega results may vary.

Test 4 and 7 will yield unexpected results. Why? You are adding a BYTE variable to another type of variable, and, then you are passing this calc to a GLCD method that expects a long. But, of course tests 5 and 6 work - because you are helping the compiler to reduce the complexity by for test 5 -#1 using a variable of a known type ( myTemp) and for test 6 - putting the Word variable first.

There may be ways to use casting to resolve this but this is good enough to explain the issue.

My rule - Do not do calculations as a parameter when using different types of variables.

dim var1 (10) as byte
dim var2 (10) as word
dim var3 (10) as long
dim myTemp as word
var1 (1)=100:var2 (2)=1000:var3 (3)=100000

//! 1
glcdprint (0,0,1)
glcdprint (10,0,var1(1))

//! 2
glcdprint (0,8,2)
glcdprint (10,8,var2(2))

//! 3
glcdprint (0,16,3)
glcdprint (10,16,var3(3))

//! 4
glcdprint (0,24,4)
glcdprint (10,24,(var1(1)+var2(2)))

//! 5
myTemp = var1(1) + var2(2)
glcdprint (0,32,5)
glcdprint (10,32,myTemp)

//! 6
glcdprint (0,40,6)
glcdprint (10,40,(var2(2)+var1(1)))

//! 7
glcdprint (0,48,7)
glcdprint (10,48,(var1(1)+var2(2)+var3(3)))
// glcdprint (10,48,(var3(3)+var2(2)+var1(1)))

//! 8
glcdprint (0,56,8)
glcdprint (10,56,(myTemp+var3(3)))

---

And, then try this... The multiplication of 1 adds no code but forces the compiler to create a temp variable of type Word that is the correctly add to var.

//! 4
glcdprint (0,24,4)
glcdprint (10,24,(var1(1)*[WORD]1)+var2(2))

[stan cartwright]

Thanks for replying. I tried
dim total as long
total=var1(1)+var2(2)+var3(3)
glcdprint (0,0,str(total))
still got 35564
I don't understand casting. Is that var as something?

I tried
dim var1 (10) as long
dim var2 (10) as long
dim var3 (10) as long
dim total as long
var1 (1)=100:var2 (2)=1000:var3 (3)=100000
total=var1(1)+var2(2)+var3(3)

glcdprint (0,0,str(total))

still got 35564

I tried
dim var1 (10) as word
dim var2 (10) as word
dim var3 (10) as word
dim total as word
var1 (1)=100:var2 (2)=1000:var3 (3)=10000
total=var1(1)+var2(2)+var3(3)
GLCDRotate ( Portrait ) ;xy=0 top left,
;optionally you can rotate the screen.GLCDRotate ( Portrait_Rev )
GLCDCLS bk ;black background
glcdprint (0,0,str(total))

and got 11100

does it mean glcdprint (0,0,str(longl)) don't work?

[Anobium]

Try.... one of these. GLCDPrint (long) or GLCDPrint (Str32(long))

GLCDPrint 0,0, total
GLCDPrint (0,8,str32(total))

[stan cartwright]

GLCDPrint (0,0,str32(total)) sorted printing long
dim var1 (10) as long
dim var2 (10) as long
dim var3 (10) as long
var1 (1)=100:var2 (2)=1000:var3 (3)=10000
total=var1(1)+var2(2)+var3(3)
dim total as long
glcdprint (0,0,str32(total))

now prints11100

tried
dim var1 (10) as byte
dim var2 (10) as word
dim var3 (10) as long
dim total as long
var1 (1)=100:var2 (2)=1000:var3 (3)=100000
total=var1(1)+var2(2)+var3(3)
glcdprint (0,0,str32(total))

got 101100

[Anobium]

All good. :-)

[stan cartwright]

Yes good but where was the original problem? It seemed about array elements. Seems different types can added if the result is the largest type ie long.
but mixing them in a logical expression is not recommended?
the compiler is not psychic.

I am digressing now... what do people think about goto? I can think it's got uses like exiting for next if no more to do... but now I use repeat loops more so digress further:)

[Anobium]

First paragraph is a good summary.

I am ignoring the second paragraph. lol

[Domenic Cirone]

This precedence should be in the documentation, might be usful. Including the comment using brackets to aid the compiler.

[stan cartwright]

@Domanic...I say brackets not braces but...
I think always use braces to be safe. It makes code easier to read if we follow the same idea of priorities. What's in braces done first then multiply then divide, add, subtract but does gcb do this and are braces in the flashed code? I don't understand the asm files generated so don't know.

[Anobium]

I used braces interchangeably with brackets. I know I this is incorrect - sorry.

[Anobium]

@Domenic

Please see http://gcbasic.sourceforge.net/help/_setting_variables.html

[David Stephenson]

I too have had issues with mixed expressions. I generally use "casting" to eliminate the problem.

[stan cartwright]

@Domenic.. I thought 0 was false and that 1 was true, not true=255.

= (equal)
<> (not equal)
< (less than)
> (greater than)
<= (less than or equal)
>= (more than or equal)

The final six operands are for checking conditions. They will return FALSE (0) if the condition is false, or TRUE (255) if the condition is true.

And now we definitely know dim var as byte defines it a value of zero.

I use and for masking vars not if var1=1 and var2=10 then because if var1=1 fails it still tests var2=10.
if var1=1 then if var2=10 then is better as if var1=1 fails it won't test var2=10

[Anobium]

@Stan

The final six operands are for checking conditions. They will return FALSE (0) if the condition is false, or TRUE (255) if the condition is true

The final six operands are for checking conditions. They will return FALSE (0) if the condition is false, or TRUE (255) if the condition is true. And, therefore you can check the return is 1 (as bit 0 of 255 is 1 when TRUE)

And now we definitely know dim var as byte defines it a value of zero.

Only on very new AVRs. Old AVR and PICs do not clear the RAM on some resets. This is my experience. Always init your vars to a value.

[stan cartwright]

from http://gcbasic.sourceforge.net/help/_setting_variables.html
Explanation:
Variable will be set to data.
data can be either a fixed value (such as 157), another variable, or a sum.
All unknown byte variables are assigned Zero. A variable with the name of Forever is not defined by Great Cow BASIC and therefore defaults to the value of zero.

So I always use option explicit, dim all vars and initialise them and use brackets to show expression evaluation order. I don't assume anything the compiler does.
I don't use expression true/false logic, I just never got into it... my loss?
I don't have problems with gcb cos my code is simple and the compiler gets on with it.

if (match = (times_array(1) + 2) >= times_array(3)) >0 then it's true?

[Anobium]

All unknown byte variables are assigned Zero

Do not assume that on non LGT or very new AVR chips.

---

The rest of your approach is very sound,