From: Jan W. <we...@ef...> - 2007-10-24 07:53:27
|
I did not want to post this as a bug, as I am not sure whether this is not just me... :-) Isn't at least some of the following supposed to be optimised into a djnz? Only the last one did, although it seems to me that some of the others could be managed similarly... Jan Waclawek --- compiled with SDCC : mcs51/gbz80/z80/avr/ds390/pic16/pic14/TININative/xa51/ds400/hc08 2.7.4 #4932 (Oct 15 2007) (MINGW32) --- compiled as sdcc p1.c --- p1.c unsigned char cnt, i; void main(void) { unsigned char cnt2; for (cnt = 0; cnt < 10; cnt++) i++; while (cnt--) i++; do i++; while (--cnt); for (cnt2 = 0; cnt2 < 10; cnt2++) i++; while (cnt2--) i++; do i++; while (--cnt2); } --- part of p1.asm ; p1.c:5: for (cnt = 0; cnt < 10; cnt++) i++; mov _cnt,#0x0A 00115$: inc _i dec _cnt mov a,_cnt jnz 00115$ mov _cnt,#0x0A ; p1.c:6: while (cnt--) i++; 00101$: mov r2,_cnt dec _cnt mov a,r2 jz 00104$ inc _i ; p1.c:7: do i++; while (--cnt); sjmp 00101$ 00104$: inc _i dec _cnt mov a,_cnt jnz 00104$ ; p1.c:9: for (cnt2 = 0; cnt2 < 10; cnt2++) i++; mov r2,#0x0A 00118$: inc _i djnz r2,00118$ ; p1.c:10: while (cnt2--) i++; mov r2,#0x0A 00107$: mov ar3,r2 dec r2 mov a,r3 jz 00131$ inc _i ; p1.c:11: do i++; while (--cnt2); sjmp 00107$ 00131$: 00110$: inc _i djnz r2,00110$ ret --- |
From: Maarten B. <sou...@ds...> - 2007-10-24 08:20:42
|
Jan, > I did not want to post this as a bug, as I am not sure whether this is > not just me... :-) IMHO it's no bug. The code performs what it's supposed to do. But you may classify this as a Request For Enhancement. Maybe it even already is. > Isn't at least some of the following supposed to be optimised into a > djnz? Only the last one did, although it seems to me that some of the > others could be managed similarly... Btw. I see DJNZ twice ;-) But you may recompile this with -fverbose-asm to also view the iCodes and the used peephole rules. These can point to places where optimizations can be extended. Greets, Maarten |
From: Jan W. <we...@ef...> - 2007-10-24 22:06:47
|
> Btw. I see DJNZ twice ;-) OK, I can't count up to two. Time for supplemental education maybe... :-) I just thought I've seen this optimised before and thought about something going wrong in the peephole rules (it happens, doesn't it). But, meantime, I managed to install version 2.6.0 parallel to 2.7.0 and the actual snapshot (which is kind of tricky as the installer insists on uninstalling other ver, thanks god it is stupid enough to try to clear /ProgramFiles/SDCC only, regardless on the actual directory where the previous version was installed), and it performed in the same way in all three versions; so it was just me. While the peephole optimiser seems to be an excellent idea at the first sight, I am afraid it strikes back in the long run, so I don't want to get involved in that one at the moment. Thanks for your comments and your time. Jan ----- Original Message ----- From: Maarten Brock <sou...@ds...> > Jan, > > > I did not want to post this as a bug, as I am not sure whether this is > > not just me... :-) > > IMHO it's no bug. The code performs what it's supposed to do. But you may > classify this as a Request For Enhancement. Maybe it even already is. > > > Isn't at least some of the following supposed to be optimised into a > > djnz? Only the last one did, although it seems to me that some of the > > others could be managed similarly... > > Btw. I see DJNZ twice ;-) > > But you may recompile this with -fverbose-asm to also view the iCodes and > the used peephole rules. These can point to places where optimizations can > be extended. |
From: Maarten B. <sou...@ds...> - 2007-10-26 18:25:45
|
> It was in peephole rule 257.b. I disabled it > however because in combination with peephole rule 256 it > broke things (see bug 1721024). I may have a way to > enable it again. Done by moving the rules 257.x to 253.x, i.e. before 256.x. |
From: Jan W. <we...@ef...> - 2007-10-26 19:33:16
|
>Done by moving the rules 257.x to 253.x, i.e. before >256.x. Thanks, Maarten. Jan |
From: Maarten B. <sou...@ds...> - 2007-10-26 20:17:38
|
Hi, I would like to change the regression test makefile by changing the target 'test-mcs51' to 'test-mcs51-small' and then create a new target 'test-mcs51' which depends on all 'test-mcs51-xxx' targets. This would enable me to run all the mcs51 tests, but not the tests for other processors. This is useful when only the mcs51 generator or peephole files have changed. But I don't know if this change could break anything on the compile farm. Therefor I'm reluctant to commit the change. If anyone knows, please advise. Maarten |
From: Borut R. <bor...@si...> - 2007-10-26 20:39:56
|
Maarten, the snapshot build just runs the make from support/regression directory, so this should not be a problem. Anyway, we well see the result in next snapshot build ;-) Just go for it! Borut Maarten Brock wrote: > Hi, > > I would like to change the regression test makefile by > changing the target 'test-mcs51' to 'test-mcs51-small' > and then create a new target 'test-mcs51' which depends > on all 'test-mcs51-xxx' targets. This would enable me to > run all the mcs51 tests, but not the tests for other > processors. This is useful when only the mcs51 generator > or peephole files have changed. > > But I don't know if this change could break anything on > the compile farm. Therefor I'm reluctant to commit the > change. If anyone knows, please advise. > > Maarten > |
From: Maarten B. <sou...@ds...> - 2007-10-26 20:49:06
|
Thanks Borut, > the snapshot build just runs the make from support/regression directory, > so this should not be a problem. I doubt this. The regression tests on the compile farm do not run the test-mcs51-medium and test-mcs51-xstack- auto tests. Also the order of the tested targets is different from what happens locally. > Anyway, we well see the result in next snapshot build ;-) > Just go for it! > Borut Ok, I will. Maarten |
From: Borut R. <bor...@si...> - 2007-10-27 09:18:36
|
Maarten Brock wrote: > Thanks Borut, > > >> the snapshot build just runs the make from support/regression directory, >> so this should not be a problem. >> > > I doubt this. The regression tests on the compile farm > do not run the test-mcs51-medium and test-mcs51-xstack- > auto tests. Also the order of the tested targets is > different from what happens locally. > > You are right. The list of regression tests that are executed by snapshot build is defined by REGTESTTARGETS variable in sdcc-build/lib/variables.mk. The make is executed from sdcc-regression target in sdcc-build/components/sdcc.mk. The line is very long, so I didn't see the $(REGTESTTARGETS) in it yesterday :-[ I replaced test-mcs51 with test-mcs51-small in REGTESTTARGETS. Shouldn't we also rename the support/regression/ports/mcs51 directory to support/regression/ports/mcs51-small (and fix all dependencies accordingly)? Borut |
From: Maarten B. <sou...@ds...> - 2007-10-27 10:08:39
|
Borut, > I replaced test-mcs51 with test-mcs51-small in REGTESTTARGETS. Thanks. > Shouldn't we also rename the support/regression/ports/mcs51 directory to > support/regression/ports/mcs51-small (and fix all dependencies accordingly)? We could if we prefer it for consistency, but it is not necessary. I have no problem with leaving that as-is. Maarten |