Menu

Resolved: Special case conditional expression bug

Anobium
2022-04-28
2022-04-29
  • Anobium

    Anobium - 2022-04-28

    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...

     

    Last edit: Anobium 2022-04-28
  • Anobium

    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
  • stan cartwright

    stan cartwright - 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

    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

      Anobium - 2022-04-28

      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

        stan cartwright - 2022-04-28

        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

          Anobium - 2022-04-28

          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
          • stan cartwright

            stan cartwright - 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?

             
            • Anobium

              Anobium - 2022-04-28

              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.

              :-)

               

              Last edit: Anobium 2022-04-28
              • stan cartwright

                stan cartwright - 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
                • Anobium

                  Anobium - 2022-04-29

                  @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))
                  
                   

                  Last edit: Anobium 2022-04-29
                  • stan cartwright

                    stan cartwright - 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
                    • Anobium

                      Anobium - 2022-04-29

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

                      GLCDPrint 0,0, total
                      GLCDPrint (0,8,str32(total))
                      
                       
                      • stan cartwright

                        stan cartwright - 2022-04-29

                        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

                          Anobium - 2022-04-29

                          All good. :-)

                           
                          • stan cartwright

                            stan cartwright - 2022-04-29

                            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

                              Anobium - 2022-04-29

                              First paragraph is a good summary.

                              I am ignoring the second paragraph. lol

                               
  • Domenic Cirone

    Domenic Cirone - 2022-04-28

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

     
    • stan cartwright

      stan cartwright - 2022-04-28

      @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

        Anobium - 2022-04-29

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

         
    • Anobium

      Anobium - 2022-04-29
       
  • David Stephenson

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

     
    • stan cartwright

      stan cartwright - 2022-04-29

      @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

        Anobium - 2022-04-29

        @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

          stan cartwright - 2022-04-29

          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

            Anobium - 2022-04-29

            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,

             

Log in to post a comment.

Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.