#12 add->inc optimisation

closed-accepted
Maarten Brock
None
5
2006-09-29
2004-01-29
Stas Sergeev
No

Hi.

The attached patch is a peephole rule that reduces
add to inc. It saves 9 bytes for my program - not too
much, but perhaps can be usefull for someone (provided
the patch is correct of course:)

Discussion

1 2 > >> (Page 1 of 2)
    • summary: add->inc peephole optimisation --> add->inc peephole optimisation
     
  • Logged In: YES
    user_id=589052

    Hi Stas,
    unfortunately "inc" does not change the flags register
    (neither Z nor C so f.e. a following "addc" could fail).
    This makes this peephole potentialy unsafe for 16/32 bit
    instructions, so I feel uneasy to add it.
    Maybe you can prepend or append instructions (like the mov
    a,_bp in peephole 212) to make this peephole more specific.
    - Frieder

     
  • Stas Sergeev
    Stas Sergeev
    2004-01-31

    Logged In: YES
    user_id=501371

    Thanks for reviewing, the peephole opt is a tricky business
    indeed.
    That sequence comes from genPlus, and is unlikely can
    be prepended or appended with something for peephole (looks
    self-contained).
    Perhaps the better idea would be to patch the code generator
    itself then? I've hardcoded that optimization in gen.c.
    Now it is guaranteed to work only with 8bit operands and,
    as a bonus, also replaces the addition of 2 by two
    increments.
    For consistency I also added this to genMinusDec, but I
    don't have a test-case for that, so no idea how it works.

    Maybe that new patch is more viable than the previous one?

     
  • Stas Sergeev
    Stas Sergeev
    2004-01-31

    • summary: add->inc peephole optimisation --> add->inc optimisation
     
  • Logged In: YES
    user_id=589052

    Hi,
    yes, that looks obviously correct.
    But the patch seems to trigger a bug for variable c
    (a,b and d(!) are fine) in the following example:

    xdata unsigned char a,b,c,d;
    tst()
    {
    a++;
    b--;
    c+=2;
    d-=2;
    }

    ;plusinc.c:7: c+=2;
    ; genAssign
    mov dptr,#_c
    movx a,@dptr
    ; genPlus
    ; genPlusIncr
    ; Peephole 177.d removed redundant move
    mov r2,a
    mov dptr,#_c
    add a,#0x02 <-- OK (quicker than inc a, inc a)
    inc a <-- ?
    movx @dptr,a

    Someone else should have a look...

    Frieder

     
  • Bernhard Held
    Bernhard Held
    2004-02-01

    Logged In: YES
    user_id=203539

    I already had a look at this optimization long time ago. The problem was
    that it broke a quite optimistic peephole, which is needed for bp
    calculation in --stack-auto. Fixing this peephole resulted in a much larger
    code. Therefore I didn't commit the stuff. Please run the regression tests
    (especially test-mcs51-stack-auto) before comitting anything. I'll have a
    look at my archive tomorrow.

     
  • Stas Sergeev
    Stas Sergeev
    2004-02-02

    Logged In: YES
    user_id=501371

    Hi guys.

    I was not aware that such a simple patch can be so
    controversial:) Well, at least I can try to make it better.

    > But the patch seems to trigger a bug
    I can only hope someone is up to fix it:)

    > add a,#0x02 <-- OK (quicker than inc a, inc a)
    Yes I see. My next patch does exactly this by adding a
    proper peephole rule. It no longer puts 2 incs.

    > I already had a look at this optimization long time ago.
    > The problem was that it broke a quite optimistic peephole,
    Please take a look into a new patch. Although it may still
    break the peephole you refer to, it adds another peephole
    and overall saves 25 bytes for me (which is quite
    significant after all).
    Actually 11 out of that 25 bytes are saved because I added
    a "restart" for 236.a (otherwise my new rule doesn't
    activate). Which makes me to wonder: why the every rule is
    not restarting the iteration by default? I hacked
    SDCCpeeph.c to make it always restart, and got even a
    smaller code. So why they are not restarting by default?
    Or, perhaps even better, why peephole doesn't do multiple
    iterations over the whole rule list?

     
  • Stas Sergeev
    Stas Sergeev
    2004-02-02

    Another attempt

     
    Attachments
  • Maarten Brock
    Maarten Brock
    2006-09-27

    Logged In: YES
    user_id=888171

    I just had a look at this patch and related stuff.

    1) Stas, your peephole rule 104.b seems to have little
    resemblance to 104(.a). Besides it is already implemented
    as rule 214.
    2) Peephole rule 104 doesn't seem to get triggered
    anymore. It's not used in the regression tests. Nor are
    207, 209 and 212. Bernhard are these the rules you
    referred to?
    3) The patch for genPlusIncr and genMinusDec looks ok. I'm
    running regression tests now.

    If all goes well, I'll apply the patch and remove the 4
    unused rules.

    Maarten

     
  • Maarten Brock
    Maarten Brock
    2006-09-27

    • assigned_to: nobody --> maartenbrock
     
1 2 > >> (Page 1 of 2)