#237 Implementation of feature request 400

None
closed-accepted
None
5
2014-04-21
2014-04-06
Ben Shi
No

With C code

extern int g(long, long);
extern void h(void *);

int f(void)
{
char c[16];
h(c);
return(g(0, 0));
}

now
addw sp, #8
addw sp, #16
is optimized to
addw sp, #(#8 + #16)

1 Attachments

Related

Patches: #237

Discussion

  • Ben Shi

    Ben Shi - 2014-04-09

    this patch is against r8982.

     
  • Philipp Klaus Krause

    Do you think it would be worth it to have another parameter to the arithmetic operation, so we can call it like

    immdInRange(0 255 %1 '+' %2)?

    Do you think it would be a good idea for functionimmdInRange() to provide the result of the calculation (like labelIsUncondJump() does)?

    Philipp

     
  • Ben Shi

    Ben Shi - 2014-04-12

    I hope this function can be as powerful as possible. But it also means more complex implementation.

    How about introducing another pair of lex/yacc script which implements a simple calculator ? So this function immdInRange() can be called in form like:

    (lower bound, upper bound, %1, %2, %3, +, *, %4, /)

    which means lower <= %1 * (%2 + %3) / %4 <= upper ?

     
  • Philipp Klaus Krause

    For all the use cases I can currently think of, having addition and subtraction would be enough. And if we have a variable for the result, we get the more complex stuff by the "replace restart" from the peephole optimizer.

    Philipp

     
  • Ben Shi

    Ben Shi - 2014-04-14

    I made some changes according to your suggestion:

    1. adding a new parameter for '+' / '-'
    2. adding a new parameter for the result
    3. more smart handling of the '#' prefix
     
  • Ben Shi

    Ben Shi - 2014-04-16

    The above modified c.patch only accepts decimal numbers (lower / upper bound, and the input left and right operand). Should octonary and hexadecimal ones be supported ?

    I find that sdasstm8 accepts hexadecimal numbers, but still treats numbers with 0 prefix as decmimal.

    Should this patch follow sdasstm8 or sdasstm8 be changed ?

     
  • Philipp Klaus Krause

    I'd say we want hexadeciaml, since sdcc emits them.

    Philipp

     
  • Ben Shi

    Ben Shi - 2014-04-16

    So the features of this function

    immdInRange (low, high, operator, %1, %2, %9)

    1. operator only supports '+' and '-'
    2. low / high gives the range
    3. %1 represents the left operand, %2 represents the right operand, and %9 represents the result
    4. some characters are treated as white (such as '#' , an immediate's prefix)
    5. decimal and hexadecimal (with prefix 0x) numbers are accepted, but not octonary and binary.
    6. positive and negative numbers are accepted

    Any more to supplement?

     
  • Maarten Brock

    Maarten Brock - 2014-04-16

    I don't think you can simply ignore # in an operand. Maybe it's better to change the matching pattern like this:

    replace restart {
    addw sp, #%1
    addw sp, #%2
    } by {
    addw sp, #%9
    } if immdInRange(0 255 '+' %1 %2 %9)

     
  • Ben Shi

    Ben Shi - 2014-04-18

    Update !

    This new patch's features:

    for the function immdInRange (low, high, operator, %1, %2, %9)
    1. operator only supports '+' and '-'
    2. low / high gives the range
    3. %1 represents the left operand, %2 represents the right operand, and %9 represents the result
    4. decimal and hexadecimal numbers are accepted, but no octonary and binary.
    5. positive and negative numbers are both accepted

    for the peep hole rule of stm8, Maarten's above suggestion is adopted.
    replace restart {
    addw sp, #%1
    addw sp, #%2
    } by {
    addw sp, #%9
    } if immdInRange(0 255 '+' %1 %2 %9)

     
  • Philipp Klaus Krause

    • assigned_to: Philipp Klaus Krause
    • Group: -->
     
  • Philipp Klaus Krause

    I had a look at this today, and the improvement I see is substantial: In the regression tests, the peephole optimizes in 74 places.

    I applied the patch in revision #8993 with minor modifications. In particular, I removed some error messages, since for this code:

    addw sp, #2
    addw sp, #_x

    I do not want to apply the peephole, but I don't want an error message either.

    However, immdInRange could be improved further:
    It should not rely on variables %1 and %2 to hold the operands. Instead it should take whatever is the fourth and fifth argument to the function.

    This will allow further improvements: I gave two examples in the new peepholes 13a and 13b (currently commented out).

    Philipp

     
  • Philipp Klaus Krause

     
  • Ben Shi

    Ben Shi - 2014-04-20

    your iimprovement idea is easy to implement.

    but you closed this ticket, so i create a new one for that ?

    Philipp Klaus Krause spth@users.sf.net编写:

    • Comment:

    I had a look at this today, and the improvement I see is substantial: In the regression tests, the peephole optimizes in 74 places.

    I applied the patch in revision #8993 with minor modifications. In particular, I removed some error messages, since for this code:

    addw sp, #2
    addw sp, #_x

    I do not want to apply the peephole, but I don't want an error message either.

    However, immdInRange could be improved further:
    It should not rely on variables %1 and %2 to hold the operands. Instead it should take whatever is the fourth and fifth argument to the function.

    This will allow further improvements: I gave two examples in the new peepholes 13a and 13b (currently commented out).

    Philipp


    [patches:#237] Implementation of feature request 400

    Status: open
    Group:
    Created: Sun Apr 06, 2014 07:31 AM UTC by Ben Shi
    Last Updated: Sun Apr 20, 2014 08:43 AM UTC
    Owner: Philipp Klaus Krause

    With C code

    extern int g(long, long);
    extern void h(void *);

    int f(void)
    {
    char c[16];
    h(c);
    return(g(0, 0));
    }

    now
    addw sp, #8
    addw sp, #16
    is optimized to
    addw sp, #(#8 + #16)


    Sent from sourceforge.net because you indicated interest in https://sourceforge.net/p/sdcc/patches/237/

    To unsubscribe from further messages, please visit https://sourceforge.net/auth/subscriptions/

     

    Related

    Patches: #237

  • Philipp Klaus Krause

    I closed the feature request ticket, but left this patch ticket open for further use.

    Philipp

     
  • Ben Shi

    Ben Shi - 2014-04-21

    Update ! This new patch is against reversion 9000.

    1. The new immdInRange treats the fourth and the fifth paramters as operands, and the sixth parameter as the result container. Does not restrict them to %1 %2 %9 any longer.

    2. The new immdInRange can accept operands in both direct numbers and matched patterns.

    3. Fix a bug in immdGet, where +-10 is treated as positive, which should be negitive.

    4. "+", '+', +, add, "add", 'add' are all accepted as an add operator

    5. "-", '-', -, sub, "sub", 'sub' are all accepted as an sub operator

    But there is an issue:
    Could you please remove the single quote marks in your commented out code in stm8/peeph.def?

    Changing
    //replace restart {
    // pop a
    // addw sp, #%2
    //} by {
    // addw sp, #%9
    // ; peephole 13a merged pop a into addw.
    //} if notUsed('a'), immdInRange(0 255 '+' '1' %2 %9)

    //replace restart {
    // popw x
    // addw sp, #%2
    //} by {
    // addw sp, #%9
    // ; peephole 13b merged popw x into addw.
    //} if notUsed('x'), immdInRange(0 255 '+' '2' %2 %9)

    to

    if notUsed('x'), immdInRange(0 255 '+' 2 %2 %9)
    if notUsed('a'), immdInRange(0 255 '+' 1 %2 %9)

    is better.

    Since currently immdGet can not recognize the single quote marks.

     
  • Philipp Klaus Krause

    • status: open --> closed-accepted
     
  • Philipp Klaus Krause

    Applied in revision #9003.

    Philipp

     

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

Sign up for the SourceForge newsletter:





No, thanks