#89 modulus 16 could be reduced to anding 0x0f

[Frieder Ferlemann]

modulus 16 for unsigned char arguments could be
reduced to anding 0x0f. The example below shows that
an unsigned int modulus is already reduced to the &
operation:

---------8<----------------------
volatile unsigned char res;
unsigned char x;
unsigned int y;

void main()
{
res = x % 16; // uses division instead of &0x0f
res = y % 16; // 16bit modulus uses &0x000f
}

-------->8-----------------------

;mod.c:7: res = x % 16; // uses division instead of
&0x0f
; genMod
; genModOneByte
mov b,#0x10
mov a,_x
div ab
mov _res,b
;mod.c:8: res = y % 16; // 16bit modulus uses
&0x000f
; genAnd
mov a,#0x0F
anl a,_y
mov r2,a
mov r3,#0x00
; genCast
mov _res,r2

[Hubert Sack]

Logged In: YES
user_id=1160854

The following code should produce the optimized code (I'm
not able to compile and test it, so it's only a suggestion):

/src/mcs51/gen.c:

(only the added code shown!)

....
/*-------------------------------------------------------*/
/* genModOneByte : 8 bit modulus */
/*-------------------------------------------------------*/
static void
genModOneByte (operand * left,
operand * right,
operand * result)
{
.....
.....
rUnsigned = SPEC_USIGN (getSpec (operandType (right)));

if (AOP_TYPE(right) == AOP_LIT) /* if right is a literal,
check it for 2^n */
{
unsigned char val = (char) floatFromVal (AOP (right)->
aopu.aop_lit);

switch (val)
{
case 1: /* sometimes it makes sense on tricky code...*/
case 2:
case 4:
case 8:
case 16:
case 32:
case 64:
case 128:
if (isOperandInDirSpace (result) &&
operandsEqu (result, left))
{
emitcode ("anl", "%s,#0x%02x",
aopGet (AOP (result), 0, FALSE, FALSE),
val - 1);
}
else
{
MOVA (aopGet (AOP (left), 0, FALSE, FALSE));
emitcode ("anl", "a,#0x%02x", val - 1);
aopPut (AOP (result), "a", 0,
isOperandVolatile (result, FALSE));
while (size--)
aopPut (AOP (result), zero, offset++,
isOperandVolatile (result, FALSE));
}
return;
}
}

pushedB = pushB ();
....
....

[Erik Petrich]

Logged In: YES
user_id=635249

Would it not be better to handle this optimization at the
iCode level (such as it is currently for int and long)?

Also, this kind of optimization should only be applied to
unsigned values, otherwise the result is incorrect when the
first operand is negative (the C standard requires that
(a/b)*b + a%b == a). For signed values a%b could be
optimized to (a>=0) ? (a&(abs(b)-1)) : -((-a)&(abs(b)-1)),
although it is difficult to say if this is really an
improvement. The current optimization for int and long is
incorrectly disregarding the signedness.

[Hubert Sack]

Logged In: YES
user_id=1160854

Yes, you are right. The iCode Level will be the better place.

Nevertheless I added the query for unsigned (jut before
reading the comment here), because I did some more checks
and found my mistake...
So the sorce must be (if done in gen.c):

lUnsigned = SPEC_USIGN (getSpec (operandType (left)));

/* if left is unsigned and right is a literal, check it for 2^n */
if (lUnsigned && (AOP_TYPE(right) == AOP_LIT))
{
unsigned char val = (char) floatFromVal (AOP (right)
->aopu.aop_lit);

switch (val)
{
case 1: /* sometimes it makes sense (on tricky code
and hardware)... */
....
....
rUnsigned = SPEC_USIGN (getSpec (operandType (right)));

pushedB = pushB ();

[Hubert Sack]

Logged In: YES
user_id=1160854

There is a simple solution for signed chars, too. It saves a lot
of machine cycles. There is the beginning of the function (I
stopped at the point where the original code continues)

static void
genModOneByte (operand * left,
operand * right,
operand * result)
{
bool lUnsigned, rUnsigned, pushedB;
bool runtimeSign, compiletimeSign;
symbol *lbl;
int size, offset;

D(emitcode ("; genModOneByte",""));

size = AOP_SIZE (result) - 1;
offset = 1;
lUnsigned = SPEC_USIGN (getSpec (operandType (left)));

/* if left is unsigned and right is a literal, check it for 2^n */
if (AOP_TYPE(right) == AOP_LIT)
{
unsigned char val = (char) floatFromVal (AOP (right)-
>aopu.aop_lit);

switch (val)
{
case 1: /* sometimes it makes sense (on tricky code
and hardware)... */
case 2:
case 4:
case 8:
case 16:
case 32:
case 64:
case 128:
if (lUnsigned && isOperandInDirSpace (result) &&
operandsEqu (result, left))
{
emitcode ("anl", "%s,#0x%02x", aopGet (AOP
(result), 0, FALSE, FALSE), val - 1);
}
else
{
symbol *lbl2 = NULL;

MOVA (aopGet (AOP (left), 0, FALSE, FALSE));
emitcode ("anl", "a,#0x%02x", val - 1);
if (!lUnsigned)
{
int size2 = size;
int offs2 = offset;

lbl = newiTempLabel (NULL);
emitcode ("jz", "%05d$", (lbl->key + 100));
emitcode ("orl", "a,#0x%02x", 0xff ^ (val - 1));
if (size2)
{
aopPut (AOP (result), "a", 0,
isOperandVolatile (result, FALSE));
while (size2--)
aopPut (AOP (result), "#0xff", offs2++,
isOperandVolatile (result, FALSE));
lbl2 = newiTempLabel (NULL);
emitcode ("sjmp", "%05d$", (lbl2->key +
100));
}
emitcode ("", "%05d$:", (lbl->key + 100));
}
aopPut (AOP (result), "a", 0, isOperandVolatile
(result, FALSE));
while (size--)
aopPut (AOP (result), zero, offset++,
isOperandVolatile (result, FALSE));
if (!lUnsigned && lbl2)
{
emitcode ("", "%05d$:", (lbl2->key + 100));
}
}
return;
}
}

rUnsigned = SPEC_USIGN (getSpec (operandType (right)));

pushedB = pushB ();

[Hubert Sack]

Logged In: YES
user_id=1160854

For signed operands there must be a qeury for the sign of the
source otherwise the result may be incorrect:

....
....
/* if right is a literal, check it for 2^n */
if (AOP_TYPE(right) == AOP_LIT)
{
signed char val = (char) floatFromVal (AOP (right)
->aopu.aop_lit);

val = lUnsigned ? (unsigned char)val : abs(val);

switch (val)
....
....
symbol *lbl2 = NULL;

MOVA (aopGet (AOP (left), 0, FALSE, FALSE));
if (lUnsigned)
emitcode ("anl", "a,#0x%02x", val - 1);
else
{
int size2 = size;
int offs2 = offset;

emitcode ("anl", "a,#0x%02x", (val - 1) | 0x80);
lbl = newiTempLabel (NULL);
emitcode ("mov", "c,acc.7", (lbl->key + 100));
emitcode ("clr", "acc.7");
emitcode ("jz", "%05d$", (lbl->key + 100));
emitcode ("jnc", "%05d$", (lbl->key + 100));
emitcode ("orl", "a,#0x%02x", 0xff ^ (val - 1));
if (size2)
....
....

However this code saves a lot of cycles compared to the
original...

[Hubert Sack]

Logged In: YES
user_id=1160854

My idea was o.k., but "val" is a signed char, so the cast I did
doesn't work!
It must be:

....
....
if (lUnsigned)
{
if (val < 0)
val = -val;
}
else
val = abs(val);

switch (val)
....
....

I'm so sorry...

[Hubert Sack]

Logged In: YES
user_id=1160854

A single line makes the same job...

....
....
unsigned char val = abs((char) floatFromVal (AOP (right)
->aopu.aop_lit));

switch (val)
....
....

[Maarten Brock]

Logged In: YES
user_id=888171

This could have been a bug report. The current code for
reducing 16bit modulus to ANDing doesn't check
signedness of left operand. I'm now inserting a modified
version of hsack's patch into the sources.

[Maarten Brock]

Logged In: YES
user_id=888171

Patched & fixed in SDCC 2.4.7 #902