#89 modulus 16 could be reduced to anding 0x0f

closed
None
5
2004-12-09
2004-11-12
No

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

Discussion

  • Hubert Sack

    Hubert Sack - 2004-11-23

    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

    Erik Petrich - 2004-11-24

    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

    Hubert Sack - 2004-11-24

    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

    Hubert Sack - 2004-11-24

    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

    Hubert Sack - 2004-11-26

    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

    Hubert Sack - 2004-11-26

    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

    Hubert Sack - 2004-11-26

    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

    Maarten Brock - 2004-12-07

    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

    Maarten Brock - 2004-12-07
    • priority: 1 --> 5
    • assigned_to: nobody --> maartenbrock
     
  • Maarten Brock

    Maarten Brock - 2004-12-09
    • status: open --> closed
     
  • Maarten Brock

    Maarten Brock - 2004-12-10

    Logged In: YES
    user_id=888171

    Patched & fixed in SDCC 2.4.7 #902

     

Log in to post a comment.

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:





No, thanks