From: Raphael N. <rn...@we...> - 2008-03-22 13:23:17
Attachments:
codesize-opt.patch
|
Hi, Anton Strobl just reported a code size increase of 5% for his project. I tracked the issue down to the fix to src/SDCCicode.c in r4947: Previously, function calls such as `rs232_tx("hello", 5);' compiled down to the code size- and register-efficient MOVLW 5 MOVWF POSTDEC1 MOVLW UPPER(__str1) MOVWF POSTDEC1 MOVLW HIGH(__str1) MOVWF POSTDEC1 MOVLW LOW(__str1) MOVWF POSTDEC1 CALL _rs232_tx Afterwards the result is more like MOVLW UPPER(__str1) MOVWF r0x01 MOVLW HIGH(__str1) MOVWF r0x02 MOVLW LOW(__str1) MOVWF r0x03 [...] MOVLW 5 MOVWF POSTDEC1 MOVWF r0x01 MOVWF POSTDEC1 MOVWF r0x02 MOVWF POSTDEC1 MOVWF r0x03 MOVWF POSTDEC1 CALL _rs232_tx due to the requirement of casting the generic pointer after taking a symbol's address. I have a fix (see attached diff) incorporated into the PIC16 dataflow-based pCode-optimizer, which is turned off by default and can be enabled using --optimize-df, so the fix will not by default affect all users---except for the library, which is also built using all optimizations and looks pretty good with the patch. Regression tests with --optimize-df pass, affected code looks good. I have sent the patch to Anton and am awaiting his response (obviously there will be no commit if he states that it breaks his project). For me it would be fine to hold this de-regression back till after the release, though more users would profit from it being in (of course). I ask for approval/disapproval/comments. Regards, Raphael By the way, PIC14 is not affected and generates code along the lines of the inefficient version above in all cases. |
From: Maarten B. <sou...@ds...> - 2008-03-22 13:33:42
|
Hi Raphael, You can also look at the function getPtrType() that I created in SDCCicode.c this week. If you add a case for the PICs just like I did for Z80 it will return to the old behaviour. But since I don't even know about the PIC architecture I do not know if this will break anything or not. Maybe you will need different conditions than mcs51/ds390. Maarten > Hi, > > Anton Strobl just reported a code size increase of 5% for his project. > > I tracked the issue down to the fix to src/SDCCicode.c in r4947: > Previously, function calls such as `rs232_tx("hello", 5);' compiled down > to the code size- and register-efficient > > MOVLW 5 > MOVWF POSTDEC1 > MOVLW UPPER(__str1) > MOVWF POSTDEC1 > MOVLW HIGH(__str1) > MOVWF POSTDEC1 > MOVLW LOW(__str1) > MOVWF POSTDEC1 > CALL _rs232_tx > > Afterwards the result is more like > > MOVLW UPPER(__str1) > MOVWF r0x01 > MOVLW HIGH(__str1) > MOVWF r0x02 > MOVLW LOW(__str1) > MOVWF r0x03 > [...] > MOVLW 5 > MOVWF POSTDEC1 > MOVWF r0x01 > MOVWF POSTDEC1 > MOVWF r0x02 > MOVWF POSTDEC1 > MOVWF r0x03 > MOVWF POSTDEC1 > CALL _rs232_tx > > due to the requirement of casting the generic pointer after taking a > symbol's address. > > I have a fix (see attached diff) incorporated into the PIC16 > dataflow-based pCode-optimizer, which is turned off by default and can > be enabled using --optimize-df, so the fix will not by default affect > all users---except for the library, which is also built using all > optimizations and looks pretty good with the patch. > Regression tests with --optimize-df pass, affected code looks good. > I have sent the patch to Anton and am awaiting his response (obviously > there will be no commit if he states that it breaks his project). > > For me it would be fine to hold this de-regression back till after the > release, though more users would profit from it being in (of course). > > > I ask for approval/disapproval/comments. > > Regards, > Raphael > > > By the way, PIC14 is not affected and generates code along the lines of > the inefficient version above in all cases. |
From: Raphael N. <rn...@we...> - 2008-03-22 14:57:06
Attachments:
codesize-opt-alt.patch
|
Hi Maarten, hi Borut, > You can also look at the function getPtrType() that I > created in SDCCicode.c this week. If you add a case for > the PICs just like I did for Z80 it will return to the > old behaviour. But since I don't even know about the PIC > architecture I do not know if this will break anything > or not. Maybe you will need different conditions than > mcs51/ds390. Right, I prepared an alternative patch (attached), which restores the behaviour of the pre-4947 SDCC version for the PIC16 port. Thank you, Maarten, for pointing this out. This solution might be better for the release as it is less intrusive and seemingly there are no experienced errors with the `old' approach. On the long run I think the PIC port(s) can benefit from the new/correct approach, making the type of the resulting pointer explicit (probably we can then emit the correct generic pointer tag more easily?). Still/again awaiting Borut's decision. Regards, Raphael |
From: Borut R. <bor...@si...> - 2008-03-22 15:08:04
|
Let do it. But I hope that this will be the last change, since tomorrow I'll prepare the RC2 build. Borut Raphael Neider wrote: > Hi Maarten, > hi Borut, > >> You can also look at the function getPtrType() that I created in >> SDCCicode.c this week. If you add a case for the PICs just like I did >> for Z80 it will return to the old behaviour. But since I don't even >> know about the PIC architecture I do not know if this will break >> anything or not. Maybe you will need different conditions than >> mcs51/ds390. > > Right, I prepared an alternative patch (attached), which restores the > behaviour of the pre-4947 SDCC version for the PIC16 port. Thank you, > Maarten, for pointing this out. > > This solution might be better for the release as it is less intrusive > and seemingly there are no experienced errors with the `old' approach. > On the long run I think the PIC port(s) can benefit from the > new/correct approach, making the type of the resulting pointer > explicit (probably we can then emit the correct generic pointer tag > more easily?). > > Still/again awaiting Borut's decision. > > Regards, > Raphael |
From: Raphael N. <rn...@we...> - 2008-03-22 18:11:09
|
> Let do it. OK, I just committed the small fix as r5114. > But I hope that this will be the last change, since tomorrow I'll > prepare the RC2 build. Looking forward to it. Regards, Raphael |