## Issue with modsint?

Help
2012-04-03
2013-03-12
• 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) ...
}
```

Works

```#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
00129\$:
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\$
```

Works

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

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