Learn how easy it is to sync an existing GitHub or Google Code repo to a SourceForge project! See Demo

Close

#22 Inefficient multiply by a constant

closed
Bernhard Held
None
5
2004-02-19
2002-09-01
Wim Lewis
No

The following C code:

xdata at 0x7FC6 char FOO;
char fun(unsigned char tmp)
{
tmp = 2 * (tmp-1);
return ((xdata char *)(&FOO))[tmp];
}

is compiled inefficiently by the version of SDCC I got
from CVS on 1-September-2002.

My older copy of SDCC (2.2.1, which I'm trying to
upgrade from) generates reasonably efficient code
involving a decrement and an add. The new version
(2.3.2-devel) generates much more elaborate code using
an actual MUL instruction, instead of optimizing the
MUL to an ADD.

Discussion

  • Johan Knol
    Johan Knol
    2003-04-19

    • labels: 101550 -->
     
  • Maarten Brock
    Maarten Brock
    2004-01-22

    Logged In: YES
    user_id=888171

    Dear developers,

    Tried this with SDCC 2.3.7 (2004-01-21) and it uses no MUL,
    only ADD A,ACC and RLC A along with the necessary MOV's.
    Produced code looks perfectly ok to me, although more
    optimizations are always possible.

    I think this RFE can be closed.

    Maarten

     
  • Logged In: YES
    user_id=589052

    Hi Maarten,

    > , although more optimizations are always possible.

    what you're probably alluding to is the sequence with
    the redundant xch xch pair:

    ; genLeftShift
    ; genLeftShiftLiteral
    ; genlshTwo
    mov a,r4
    xch a,r3
    add a,acc
    xch a,r3
    rlc a
    mov r4,a

    There is a peephole optimization to catch a similar
    case (peephole 252 for variables in direct data).
    Unfortunately it doesn't catch this one...
    While another peephole could be defined this is probably
    not the proper level to do it: there would have to
    be one for xdata, idata, and yes, some people would
    even want to left shift a long int too:)

    > , although more optimizations are always possible.

    Yeah, there is another one. And it would make sense
    to find a peephole optimization for it :^)

    This is it:

    mov r4,#0x00
    ; genLeftShift
    ; genLeftShiftLiteral
    ; genlshTwo
    mov a,r4

     
  • Maarten Brock
    Maarten Brock
    2004-01-23

    Logged In: YES
    user_id=888171

    Hello Frieder,

    You look with the eyes of the peephole optimizer. What I
    meant was more fundamental. This uses several temperary
    stores which it could do without.

    ; genReceive
    ;test.c:4: tmp = 2 * (tmp-1);
    ; genMinus
    ; genMinusDec
    ; Peephole 244.a moving first to a instead of r2
    mov a,dpl
    mov r2,a ;REDUNDANT, never read back
    dec a
    mov r3,a ;REDUNDANT
    ; genCast
    mov r4,#0x00 ;REDUNDANT
    ;thrown away by next cast
    ;can also be postponed
    ; genLeftShift
    ; genLeftShiftLiteral
    ; genlshTwo
    mov a,r4 ;REDUNDANT
    xch a,r3 ;REDUNDANT
    add a,acc
    xch a,r3 ;REDUNDANT (1)
    rlc a ;REDUNDANT, or at least
    ;postpone CLR A to here
    mov r4,a ;REDUNDANT, thrown away
    ; genCast
    mov ar2,r3 ;REDUNDANT (1)

    ;test.c:5: return ((xdata char *)(&FOO))[tmp];
    ; genPlus
    mov a,#0xC6 ;REDUNDANT (1)
    ; Peephole 236.a used r2 instead of ar2
    add a,r2 ;OPTIMISE (1)
    add a,#0xC6 ;OPTIMIZED (1)
    mov dpl,a
    ; Peephole 240 use clr instead of addc a,#0
    clr a
    addc a,#0x7F
    mov dph,a

    As you can see, this piece of code could have been 11 bytes
    shorter than the 30 it takes now. If you still want to be able
    to debug the code, storing in R2 would be nice but the rest
    can be optimized. Or can we even allocate local variables to
    A?

    Well, enough said. This is not what this RFE was about in the
    first place.

    Greetings,
    Maarten

     
  • Bernhard Held
    Bernhard Held
    2004-02-19

    • assigned_to: nobody --> bernhardheld
    • status: open --> closed
     
  • Bernhard Held
    Bernhard Held
    2004-02-19

    Logged In: YES
    user_id=203539

    Fixed at least in 2.3.8.