This discussion has been locked.
You can no longer post new replies to this discussion. If you have a question you can start a new discussion

C51 V6.20 & V6.21 still have reported bug

On Nov. 11, 2001, I sent in a C51 V6.20 bug report via e-mail to support.us@keil.com. I never received a response, so assumed Keil's policy was to silently introduce the bug fix in a subsequent update. After downloading V6.21, I see that the bug is still present. Now I'm wondering whether my e-mailed bug report was ever acted upon, so I'm going to try the forum as the means to report the bug.

I have a section of code with pointer incrementation that compiles and executes correctly when the pointer is defined as a generic pointer or as an xdata pointer, or when the optimization setting is reduced from the default OT(8,SPEED) to OT(7,SPEED). However, when the pointer is defined as a data or idata pointer, the compiler does not emit code to increment the pointer with default optimization.

I have provided example code demonstrating the problem. The specific statement is line 50 below.

unsigned char idata u64[8] = { 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF };
unsigned char idata s[21];

void U64ToStr( char idata *s, unsigned char idata *binP, unsigned char n )
{
    unsigned char        bcd;
    unsigned char idata *bcdP;
    unsigned char        bitMsk = 0x80;
    unsigned char        carry;
    unsigned char        i;

    /*  Zero the target buffer (NUL-terminating the string).
     */
    for (bcdP = s, i = n + 1; i != 0; *bcdP++ = 0, i--)
        ;

    /*  Convert 64 bits of binary to unpacked BCD.  For each bit, starting
     *  with the MSBit, BCD = (BCD * 2) + bit.
     */
    for (i = 64; i != 0; i--)
    {
        carry = (*binP & bitMsk) ? 1 : 0;

        for (bcdP = s + n; bcdP != s; /* Iter in body */)
        {
            bcd  = *--bcdP * 2;
            bcd += carry;

            if (bcd < 10)
                carry = 0;
            else
            {
                carry = 1;
                bcd  -= 10;
            }

            *bcdP = bcd;
        }

        if ((bitMsk >>= 1) == 0)
        {
            bitMsk = 0x80;
            binP++;
        }
    }

    /*  ASCII'fy BCD.
     */
    for (i = n; i != 0; i--)
        *bcdP++ += '0'; /* <-- C51 BUG: bcdP doesn't increment for (i)data */
}

void main( void )
{
    for (;;)
        U64ToStr( s, u64, 6 );
}

--Dan Henry

  • It's not a bug, its a great optimization. Think about, does anyone one look at bcdP after this loop? No. So why emit code to update this variable? To better understand this add

       i = *bcdP;
    
    after the loop. In this case no code what so ever is generated! So when debugging use an lower optimization level or mark the variable volatile.

  • Huh??? Great optimization? So you are suggesting that rather than adding '0' (0x30) to ASCII'fy the first 6 digits in my string like I want, it's OK for the compiler to emit code that adds '0' 6 times to only first digit? I don't understand.

  • The ptr variable is store in a register and that is incremented.

  • The ptr variable is store in a register and that is incremented.

    Right and wrong. Jon, you really must try it to see. Here's an excerpt from the compiled code:

                                               ; SOURCE LINE # 15
    ;---- Variable 'bcdP' assigned to Register 'R5' ----
    
    ;=== MOST CODE SNIPPED ===
    
                                               ; SOURCE LINE # 50
    0063 AC01              MOV     R4,AR1
    0065         ?C0014:
    0065 EC                MOV     A,R4
    0066 600B              JZ      ?C0017
                                               ; SOURCE LINE # 51
    0068 AF05              MOV     R7,AR5
    006A A807              MOV     R0,AR7
    006C 7430              MOV     A,#030H
    006E 26                ADD     A,@R0
    006F F6                MOV     @R0,A
    0070 1C                DEC     R4
    0071 80F2              SJMP    ?C0014
    
    

    I don't see any pointer (register) incrementation. What does your compiler output look like?

  • Sorry, the file that I copied the code to compile had a optimization setting different than the project. I now get the same results as you.

  • Hey, no problem. Thanks for your consideration in even looking at the problem. You just had me sweatin' there for a little while. I would be embarassed to submit a false alarm. I just hope the report gets seen this time, since e-mailing it is (apparently) the wrong means.

    Thanks again,

    --Dan Henry

  •     /*  ASCII'fy BCD.
         */
        for (i = n; i != 0; i--)
            *bcdP++ += '0'; /* <-- C51 BUG: bcdP doesn't increment for (i)data */

    I have had problems before with packing too much stuff into a single line - particularly the pre/post increment/decrement operators.
    I have seen C51 get this wrong before!

    Have you tried separating-out the operations; eg,
        /*  ASCII'fy BCD.
         */
        for (i = n; i != 0; i--)
        {
            *bcdP += '0';
            ++bcdP;
        }

    BTW: I have found that the above is not necessarily more efficient than, eg,
        for (i = n; i != 0; i--)
        {
            bcdP[i] += '0';
        }
    You would have to check carefully with the generated assembler to verify which is really the best in your particular application!

  • Thanks for your thoughts Andrew.

    Regarding "I have had problems before with packing too much stuff into a single line - particularly the pre/post increment/decrement operators.", the compiler handles the statement fine if I change "binP" to "s":

        for (i = n; i != 0; i--)
            *s++ += '0';    /* Using 's' does work. */
    
    ;---- Variable 's' assigned to Register 'R7' ----
    ;---- Variable 'i' assigned to Register 'R4' ----
    
    0061 AC00        R     MOV     R4,n
    0063         ?C0014:
    0063 EC                MOV     A,R4
    0064 600C              JZ      ?C0017
    0066 AE07              MOV     R6,AR7
    0068 0F                INC     R7       ; <-- ***
    0069 A806              MOV     R0,AR6
    006B 7430              MOV     A,#030H
    006D 26                ADD     A,@R0
    006E F6                MOV     @R0,A
    006F 1C                DEC     R4
    0070 80F1              SJMP    ?C0014
    

    As for "separating-out the operations", interestingly, the bug is still present:

        for (i = n; i != 0; i--)
        {
            *bcdP += '0';
            ++bcdP;
        }
    
    ;---- Variable 'n' assigned to Register 'R1' ----
    ;---- Variable 'bcdP' assigned to Register 'R5' ----
    ;---- Variable 'i' assigned to Register 'R4' ----
    
    0063 AC01              MOV     R4,AR1
    0065         ?C0014:
    0065 EC                MOV     A,R4
    0066 6009              JZ      ?C0017
    0068 A805              MOV     R0,AR5
    006A 7430              MOV     A,#030H
    006C 26                ADD     A,@R0
    006D F6                MOV     @R0,A
    006E 1C                DEC     R4
    006F 80F4              SJMP    ?C0014
    0071         ?C0017:
    

    With optimization reduced by one (to 7) both of our snippets compile correctly. Here's the output of the separated-out version with optimization lowered to 7:

    ;---- Variable 'n' assigned to Register 'R1' ----
    ;---- Variable 'bcdP' assigned to Register 'R5' ----
    ;---- Variable 'i' assigned to Register 'R4' ----
    
    0064 AC01              MOV     R4,AR1
    0066         ?C0014:
    0066 EC                MOV     A,R4
    0067 600A              JZ      ?C0017
    0069 A805              MOV     R0,AR5
    006B 7430              MOV     A,#030H
    006D 26                ADD     A,@R0
    006E F6                MOV     @R0,A
    006F 0D                INC     R5       ; <-- ***
    0070 1C                DEC     R4
    0071 80F3              SJMP    ?C0014
    0073         ?C0017:
    

  • Is the lack of response to my Forum-posted bug report on 12/7 due to my Dec. 2001 support expiration?

    This is certainly unfortunate timing -- trying to debug buggy client hardware, while trying to debug my software, while trying to help with an RTOS port to the Keil C51 toolchain. Now with unresolved compiler questions, it makes for an interesting exercise of "who done it?" when something does not work.

    I now realize that I've brought the lack of attention to my report upon myself by not anticipating the end date for my support and that I wouldn't be eligible for any bug fixes now (even though I originally reported the bug one month ago on 11/11), but I'd think the original e-mailed report, which was within my support date parameters, would have at least qualified for an acknowledgement of receiving the report.

    I'll be renewing support again after I dredge up records and locate my Sales contact, and will try reporting the bug again then (sigh). (Hint to Keil Sales: Professional C51 users [not kit level, but those who are in the embedded business] would likely welcome a kindly reminder that their support is expiring.)

    The lack of any response to both email-posted and Forum-posted bug reports still makes me wonder which is the proper means for reporting bugs. I seem to recall seeing a Keil response to someone's bug report on the Forum. I prefer to use private e-mail first, but has anybody had success e-mailing bug reports? Experiences/recommendations anyone?

    Frustratedly yours,

    --Dan Henry