Issue with modsint?

  • Travis Rothlisberger

    I ran into an issue with some code that takes a modulus and I wanted to get some expert insight into what might be happening.  In short, I can break or fix the code by changing the way I do a modulus calculation.

    Platform:  SiLabs C8051F06x, large memory model, no stack

    The modulus calculation below is intended to control whether an action is taken every 6th iteration, starting with iteration #6.

    Doesn't Work

    #define INDICES_PER_ACTION       6
    for(i = 0; i < NUM_VALUES; i++)
        uint8_t modulus = ((i+1) % INDICES_PER_ACTION);
        if(modulus == 0) ...


    #define INDICES_PER_ACTION       6
    for(i = 0; i < NUM_VALUES; i++)
        uint8_t modulus = (i % INDICES_PER_ACTION);
        if(modulus == (INDICES_PER_ACTION-1)) ...

    In the code that doesn't work, the value of modulus is usually correct, but sometimes (seemingly random) it is incorrect at a value less than the divisor.  Furthermore, the modulus result is never incorrect after the first time this loop is executed.  The latter symptom had me looking for an initialization problem for a long time but I couldn't find anything.  Finally, I reached a point where I saw the behavior correct itself by applying the code change above.  Upon reviewing the assembly, I was surprised to find a substantial difference.  The failing example calls __modsint, whereas the working example simply performs a divide and examines the remainder in B.  Here's the resulting assembly:

    Doesn't Work

    ;   src/display.c:298: uint8_t modulus = ((i+1) % INDICES_PER_ACTION);
        mov ar0,r2
        mov r6,#0x00
        inc r0
        cjne    r0,#0x00,00129$
        inc r6
        mov dptr,#__modsint_PARM_2
        mov a,#0x06
        movx    @dptr,a
        inc dptr
        clr a
        movx    @dptr,a
        mov dpl,r0
        mov dph,r6
        push    ar7
        push    ar4
        push    ar3
        push    ar2
        push    ar1
        lcall   __modsint
        mov r5,dpl
        mov r6,dph
        pop ar1
        pop ar2
        pop ar3
        pop ar4
        pop ar7
        mov dptr,#_TestFunction_modulus_3_3
        mov a,r5
        movx    @dptr,a
        C$display.c$299$1$1 ==.
    ;   src/display.c:299: if(modulus == 0)
        mov a,r5
        jnz 00102$


    ;   src/display.c:298: uint8_t modulus = (i % INDICES_PER_ACTION);
        mov b,#0x06
        mov a,r2
        div ab
        mov r0,b
        mov dptr,#_TestFunction_modulus_3_3
        mov a,r0
        movx    @dptr,a
        C$display.c$299$3$3 ==.
    ;   src/display.c:299: if(modulus == (INDICES_PER_ACTION-1))
        cjne    r0,#0x05,00102$

    Can anybody explain what might be happening in the broken code?  It looks like the "+ 1" causes the difference in the technique (because it treats the sum as signed and can't just use DIV?) but that doesn't explain why it breaks.  I hate implementing fixes without understanding why.  If anybody has insight into the issue with the code that uses modsint I would appreciate it.  Thanks.

  • Raphael Neider

    Raphael Neider - 2012-04-03

    I guess i is an uint8_t as well. i+1 promotes this to a (16-bit!, signed?) integer, which calls for a helper routine to implement the 16-bit division. These are C rules, and SDCC cannot do anything about it ((255+1) % 6 should be 256%6, not 0%6).
    You might want to use "((uint8_t)(i+1)) % 6" and see what happens.

    Still, the second approach should work, so you might want to file a bug report in the bug tracker - unless …
    Are any interrupt routines also using division/modulus? Judging by the use of "__modsint_PARM_2", the library routines seem to be compiled non-reentrant, so you need to avoid interrupts interfering with your main program on shared resources such as the parameters, which are being passed in globals (and dpl/dph) here.

  • Travis Rothlisberger

    Thank you very much for the reply.  You're correct that 'i' is uint8_t.  It was the second piece of information you provided that was the key.  Section of the SDCC manual explains the issue in detail.  This issue could be very dangerous and easily missed in testing.  I had 32-bit integer division in my interrupt routine, which is why I observed the issue.

    I'm curious as to why the default libraries are non-reeentrant?  Is the performance penalty so great that it outweighs the benefit of avoiding bugs that are easily introduced and difficult to diagnose?  It seems to me like the default behavior should be to provide reentrant libraries and let somebody recompile if they need the performance improvement.

    I'm actually very glad that I stumbled upon this because it could have been a real problem if I hadn't detected it earlier.  Thanks for your help.


Log in to post a comment.