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 1Iftimes_array(1)+2>=times_array(3)Thenmatch=1Elsematch=0EndIf
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
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
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?
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
@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.
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.
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?
Last edit: stan cartwright 2022-04-29
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
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
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
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:)
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
@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.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
@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
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
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.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
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?
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
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.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.
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.
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.
Enjoy the fix...
Last edit: Anobium 2022-04-28
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.
Last edit: Anobium 2022-04-28
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
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.
:-)
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?
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.
Last edit: Anobium 2022-04-28
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?
Hugh has done a good job on precedence. This is the way it is.
You will have to decode but } is equal or greater than.
:-)
Last edit: Anobium 2022-04-28
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
Last edit: stan cartwright 2022-04-28
@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.
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.
Last edit: Anobium 2022-04-29
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?
Last edit: stan cartwright 2022-04-29
Try.... one of these. GLCDPrint (long) or GLCDPrint (Str32(long))
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
All good. :-)
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:)
First paragraph is a good summary.
I am ignoring the second paragraph. lol
This precedence should be in the documentation, might be usful. Including the comment using brackets to aid the compiler.
@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.
I used braces interchangeably with brackets. I know I this is incorrect - sorry.
@Domenic
Please see http://gcbasic.sourceforge.net/help/_setting_variables.html
I too have had issues with mixed expressions. I generally use "casting" to eliminate the problem.
@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
@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. And, therefore you can check the return is 1 (as bit 0 of 255 is 1 when TRUE)
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.
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?
Do not assume that on non LGT or very new AVR chips.
The rest of your approach is very sound,