#64 Optimized assigns in "genDataPointerSet"

None
closed-fixed
Maarten Brock
None
3
2013-11-16
2006-05-17
Hubert Sack
No

Within "global & static initialisations" very often
0#00 is assigned. If, for example, the destination is a
"long" as member of a union, the assignment could be
done much more simple. This saves bytes (and cycles).
In some cases, the "other" generated code is optimized too.

The attached "gen.c.patch" file contains the solution
for the (over 1 year old) RFE's 1156868 and 1156978,
too. I used them since that time because I think they
are very usefull and should be implemented...

Discussion

  • Hubert Sack
    Hubert Sack
    2006-05-17

    result of running "svn di gen.c"

     
    Attachments
  • Hubert Sack
    Hubert Sack
    2006-05-17

    • priority: 5 --> 3
     
  • Hubert Sack
    Hubert Sack
    2006-05-17

    Logged In: YES
    user_id=1160854

    There was "&& (size > 1)" missing in line #10299 (after
    applying the patch) - sorry.
    Without this a byte member of a struct would be set to a
    literal value by using an extra instruction...
    The corrected "diff" is attached

     
  • Borut Ražem
    Borut Ražem
    2009-02-22

    You didn't specify the target for the patch. A quick search shows that candidates are mcs51 or ds390 (or both)?

    Borut

     
  • Hubert Sack
    Hubert Sack
    2009-02-22

    I did it for the mcs51 port. (I expect it should possible to do it the same way on ds390 and ds400 ports, may be on the other ones too)

     
  • Hubert Sack
    Hubert Sack
    2009-02-22

    • assigned_to: nobody --> borutr
     
  • Borut Ražem
    Borut Ražem
    2009-02-22

    • assigned_to: borutr --> nobody
     
  • Borut Ražem
    Borut Ražem
    2009-02-22

    Hubert,

    please don't assign bugs to developers.

    Maarten, do you think this is a candidate for the 2.9.0 release? Do you have time to look at it?

    Borut

     
  • Maarten Brock
    Maarten Brock
    2010-09-11

    • assigned_to: nobody --> maartenbrock
     
  • Maarten Brock
    Maarten Brock
    2010-09-11

    I finally took the time to look into this patch and decided not to use it for genDataPointerSet, but instead factor out the similar optimization from genAssign and use that instead. This way the code is more consistent and also optimizes non-zero values. Implemented in SDCC 2.9.7 #5958.

    I'm still looking at the other two parts of the patch for eliminating MOV PSW and better genCmp.

    I hope I haven't disappointed you Hubert and I still thank you for providing the patch. If nothing else it pointed me to the inefficiency of genDataPointerSet for literals.

     
  • Hubert Sack
    Hubert Sack
    2010-09-11

    I'm not disappointed. I'm happy to see that my ideas are wellcome and help SDCC to become more and more perfect.
    I added my sourcecode (original with the german comments...) and the resulting RST files. (the one of Rev. 5958 and "mine")
    There are only 2 differences left
    - the mov psw,#...
    - and the multiple clr a

    As maarten already wrote, he is still looking at the part or eliminating MOV
    PSW,#...
    As I think the multiple clr a in my RST file can be optimized away by some more peepholes

     
  • Hubert Sack
    Hubert Sack
    2010-09-11

    My source file, the RST file and the SDCC calling params

     
    Attachments
  • > As I think the multiple clr a in my RST file can be optimized away by some more peepholes

    You probably don't yet have set the (undocumented) environment variable "SDCC_REGTRACK"?

    This should remove most of the redundant "clr a".

    (The plan would be to make "SDCC_REGTRACK" default some time after SDCC 3.0.0
    and then have an compiler option for disable that feature).

     
  • Hubert Sack
    Hubert Sack
    2010-09-15

    why line 382 is not affected is not clear to me

     
    Attachments
  • Hubert Sack
    Hubert Sack
    2010-09-15

    OK - it works for a lot of times. But there is a line (382) left to be changed by the RTRACK...
    I attached the RST file - the source was already in the ZIP attachment

     
  • > But there is a line (382) left to be changed by the RTRACK...

    yes, but it's probably not representative that this is only
    one occurence:)

    Within the regression tests (for test-mcs51-small) rtrack optimizes
    404 operations and "suggests" to optimize
    407 operations...

    The excuse is, that the register tracking stuff is separated into
    two parts.

    The first part only tracking values ( rtrackUpdate() in rtrack.c
    which looks at the mnemonics passed to the assembler and
    updates its notion of register contents accordingly).

    And the second part doing the optimizing by loading
    accumulator and B if it can be done easier by known
    register contents
    ( movea() and movb() in gen.c call an rtrack function to
    get the operand cheaper).

    The second part removed the redundant clr a in the
    source you provided. So far so good.

    But the first part rtrackUpdate() also has some ideas
    on optimization. As it does not change the line it is
    passed it only "suggests" optimizations.

    trackUpdate() could (and probably should) be updated
    (and renamed) to actually perform the optimizations it
    suggests and return a pointer to a newly allocated
    optimized string.

    Unfortunately I won't have time to do this.

    Also the algorithm looses track of its registers as
    soon as a label: is seen.
    If it's state would be restored to the state of its
    dominator (term explained in manual 9.2)
    (or the common known values () of its predecessors)
    then register tracking optimization could be still
    more effective.

     
  • Maarten Brock
    Maarten Brock
    2013-11-16

    I have now enabled register tracking and added removal of suggested lines in SDCC #8897.
    And AFAIK PSW suppression is already present too.
    I therefor think this can be closed.

     
  • Maarten Brock
    Maarten Brock
    2013-11-16

    • status: open --> closed-fixed
    • Group: -->