Activity for Tobias Diedrich
-
Tobias Diedrich created ticket #492
mcs51: track register equality
-
I also have a patch for the "need pointerCode" assert, as attached.
-
Of course there were some bugs, but the regression test is passing now. $ grep "(f:" regtest-before.log | sed -e 's@.*b:\([^,]*\), T:\([^)]*\))@b: \1 T: \2@' | awk 'BEGIN { b = 0; t = 0 } { b += $2; t += $4 } END { print "b:", b, "T:", t }' b: 76397798 T: 10330885752 $ grep "(f:" regtest-after.log | sed -e 's@.*b:\([^,]*\), T:\([^)]*\))@b: \1 T: \2@' | awk 'BEGIN { b = 0; t = 0 } { b += $2; t += $4 } END { print "b:", b, "T:", t }' b: 76397020 T: 10330860444 $ grep "failures" regtest-after.log Summary...
-
$ grep "(f:" regtest-before.log | sed -e 's@.*b:\([^,]*\), T:\([^)]*\))@b: \1 T: \2@' | awk 'BEGIN { b = 0; t = 0 } { b += $2; t += $4 } END { print "b:", b, "T:", t }' b: 76390042 T: 10330159128 $ grep "(f:" regtest-after.log | sed -e 's@.*b:\([^,]*\), T:\([^)]*\))@b: \1 T: \2@' | awk 'BEGIN { b = 0; t = 0 } { b += $2; t += $4 } END { print "b:", b, "T:", t }' b: 76389574 T: 10330144896 So 468 bytes saved over all testcases?
-
$ grep "(f:" regtest-before.log | sed -e 's@.*b:\([^,]*\), T:\([^)]*\))@b: \1 T: \2@ | awk 'BEGIN { b = 0; t = 0 } { b += $2; t += $4 } END { print "b:", b, "T:", t }' b: 76390042 T: 10330159128 $ grep "(f:" regtest-after.log | sed -e 's@.*b:\([^,]*\), T:\([^)]*\))@b: \1 T: \2@ | awk 'BEGIN { b = 0; t = 0 } { b += $2; t += $4 } END { print "b:", b, "T:", t }' b: 76389574 T: 10330144896 So 468 bytes saved over all testcases?
-
Grepping over the regression test: 273 hits for "Peephole 4242.a" 120 hits for "Peephole 4242.b" 288 hits for "Peephole 4242.c" 0 hits for "Peephole hoisted temporary update out of loop" 0 hits for "Peephole optimized decrement, merged into previous" 0 hits for "Peephole removed redundant move before return" [edit]Some regressions found compared to unmodified baseline, need to check where the mov , dpl corruption is coming from... Unfortunately notSame(%1, '') is not accepted, so for now I fixed...
-
550 hits for "Peephole modified moves to enable further optimizations" That said, sampling the hits it is often only a change with no further effect (no change in size or speed). But it has some successes, for example in support/regression/gen/mcs51-medium/rotate2/rotate2_size_16_andCase_1_xorLiteral_1_rotateLeft_0_structVar_0.asm: $ diff -Naru build-{unmodified,subcarry-peeph}/support/regression/gen/mcs51-medium/rotate2/rotate2_size_16_andCase_1_xorLiteral_1_rotateLeft_0_structVar_0.asm --- build-unmodified/support/regression/gen/mcs51-medium/rotate2/rotate2_size_16_andCase_1_xorLiteral_1_rotateLeft_0_structVar_0.asm...
-
Grepping over the regression test: 550 hits for "Peephole modified moves to enable further optimizations" 0 hits for "Peephole hoisted temporary update out of loop" 0 hits for "Peephole optimized decrement, merged into previous" 0 hits for "Peephole removed redundant move before return" [edit]Some regressions found compared to unmodified baseline, need to check where the mov , dpl corruption is coming from... Unfortunately notSame(%1, '') is not accepted, so for now I fixed it by duplicating the...
-
Grepping over the regression test: 550 hits for "Peephole modified moves to enable further optimizations" 0 hits for "Peephole hoisted temporary update out of loop" 0 hits for "Peephole optimized decrement, merged into previous" 0 hits for "Peephole removed redundant move before return" [edit]Some regressions found compared to unmodified baseline, need to check where the mov , dpl corruption is coming from...[/edit] Unfortunately notSame(%1, '') is not accepted, so for now I fixed it by duplicating...
-
Grepping over the regression test: 550 hits for "Peephole modified moves to enable further optimizations" 0 hits for "Peephole hoisted temporary update out of loop" 0 hits for "Peephole optimized decrement, merged into previous" 0 hits for "Peephole removed redundant move before return" [edit]Some regressions found compared to unmodified baseline, need to check where the mov , dpl corruption is coming from...[/edit]
-
550 hits for "Peephole modified moves to enable further optimizations" That said, sampling the hits it is often only a change with no further effect (no change in size or speed). But it has some successes, for example in support/regression/gen/mcs51-medium/rotate2/rotate2_size_16_andCase_1_xorLiteral_1_rotateLeft_0_structVar_0.asm: $ diff -Naru build-{unmodified,subcarry-peeph}/support/regression/gen/mcs51-medium/rotate2/rotate2_size_16_andCase_1_xorLiteral_1_rotateLeft_0_structVar_0.asm --- build-unmodified/support/regression/gen/mcs51-medium/rotate2/rotate2_size_16_andCase_1_xorLiteral_1_rotateLeft_0_structVar_0.asm...
-
Grepping over the regression test: 550 hits for "Peephole modified moves to enable further optimizations" 0 hits for "Peephole hoisted temporary update out of loop" 0 hits for "Peephole optimized decrement, merged into previous" 0 hits for "Peephole removed redundant move before return" No regressions found compared to unmodified baseline.
-
Grepping over the regression test: 550 hits for "Peephole modified moves to enable further optimizations" 1 hit for "Peephole hoisted temporary update out of loop" 0 hits for "Peephole optimized decrement, merged into previous" 0 hits for "Peephole removed redundant move before return" No regressions found compared to unmodified baseline.
-
Grepping over the regression test: 551 hits for "Peephole modified moves to enable further optimizations" 2 hits for "Peephole hoisted temporary update out of loop" 1 hit for "Peephole optimized decrement, merged into previous" 1 hit for "Peephole removed redundant move before return" No regressions found compared to unmodified baseline.
-
Looking through mcs51/gen.c I don't think this pattern is generated anymore. Grep through the regression test results I don't see a single hit on "Peephole 131" either, so this is probably an unused rule nowadays...
-
Do you have a C code sample that can reproduce incorrect optimizations with this rule? No, I just randomly noticed this while looking for existing add/sub-related peephole rules. I had a quick look how old this rule is and it's been there for over 20 years, so its probably safe. Possibly the compiler will just never generate instructions that would trigger the false-positive case. [edit]Rule 131 goes back all the way to https://sourceforge.net/p/sdcc/code/3/[/edit]
-
Did you try them on the regression tests? Not yet, will try to do that on the weekend. Any tips? Presumably all I need to run them is make -C support/regression test-mcs51? How long is the run expected to take on a gen2 ryzen?
-
Do you have a C code sample that can reproduce incorrect optimizations with this rule? No, I just randomly noticed this while looking for existing add/sub-related peephole rules. I had a quick look how old this rule is and it's been there for over 20 years, so its probably safe. Possibly the compiler will just never generate instructions that would trigger the false-positive case.
-
Optimized strlen peephole rules: uint16_t strlen(register __code const char *s) { __code const char *x = s; while (*(x++)); return x - s - 1; } // optimized strlen replace { clr c subb a,%1 mov %1,a mov a,%2 subb a,%3 mov %3,a dec %1 cjne %1,#0xff,%4 dec %3 %4: } by { setb c subb a,%1 mov %1,a mov a,%2 subb a,%3 mov %3,a ; Peephole optimized decrement, merged into previous } if labelRefCount(%4 1) replace restart { %1: mov dpl,%2 mov dph,%3 clr a movc a,@a+dptr mov %4,a inc dptr mov %2,dpl mov %3,dph...
-
Optimized strlen peephole rules: uint16_t strlen(register __code const char *s) { __code const char *x = s; while (*(x++)); return x - s - 1; } // optimized strlen replace { clr c subb a,%1 mov %1,a mov a,%2 subb a,%3 mov %3,a dec %1 cjne %1,#0xff,%4 dec %3 %4: } by { setb c subb a,%1 mov %1,a mov a,%2 subb a,%3 mov %3,a ; Peephole optimized decrement, merged into previous } if labelRefCount(%4 1) replace restart { %1: mov dpl,%2 mov dph,%3 clr a movc a,@a+dptr mov %4,a inc dptr mov %2,dpl mov %3,dph...
-
Optimized strlen peephole rules: uint16_t strlen(register __code const char *s) { __code const char *x = s; while (*(x++)); return x - s - 1; } // optimized strlen replace { clr c subb a,%1 mov %1,a mov a,%2 subb a,%3 mov %3,a dec %1 cjne %1,#0xff,%4 dec %3 %4: } by { setb c subb a,%1 mov %1,a mov a,%2 subb a,%3 ; Peephole optimized decrement, merged into previous } if labelRefCount(%4 1) replace restart { %1: mov dpl,%2 mov dph,%3 clr a movc a,@a+dptr mov %4,a inc dptr mov %2,dpl mov %3,dph mov...
-
Optimized strlen peephole rules: // optimized strlen replace { clr c subb a,%1 mov %1,a mov a,%2 subb a,%3 mov %3,a dec %1 cjne %1,#0xff,%4 dec %3 %4: } by { setb c subb a,%1 mov %1,a mov a,%2 subb a,%3 ; Peephole optimized decrement, merged into previous } if labelRefCount(%4 1) replace restart { %1: mov dpl,%2 mov dph,%3 clr a movc a,@a+dptr mov %4,a inc dptr mov %2,dpl mov %3,dph mov a,%4 jnz %1 } by { mov dpl,%2 mov dph,%3 %1: clr a movc a,@a+dptr mov %4,a inc dptr jnz %1 ; Peephole hoisted temporary...
-
On the note of peephole rules, I think rule 131 is unsafe? replace { mov a,%1 subb a,#0x01 mov %2,a mov %1,%2 } by { ; Peephole 131 optimized decrement (not caring for c) dec %1 mov %2,%1 } AFAICS these are only identical if carry is unset at the beginning, and differ when carry is set. Maybe this was intended to be: replace { clr C mov a,%1 subb a,#0x01 mov %2,a mov %1,%2 } by { ; Peephole 131 optimized decrement (not caring for c) dec %1 mov %2,%1 }
-
I should try if this can be handled by a peephole rule instead, speaking of which it would be nice if the temporary to dptr and dptr to temporary copies could be hoisted out of the loop... And yes, both can with --peep-file containing: replace { clr c subb a,%1 mov %1,a mov a,%2 subb a,%3 mov %3,a dec %1 cjne %1,#0xff,%4 dec %3 %4: } by { setb c subb a,%1 mov %1,a mov a,%2 subb a,%3 ; Peephole optimized decrement, merged into previous %4: } replace { %1: mov dpl,%2 mov dph,%3 clr a movc a,@a+dptr...
-
I should try if this can be handled by a peephole rule instead, speaking of which it would be nice if the temporary to dptr and dptr to temporary copies could be hoisted out of the loop... And yes, both can with --peep-file containing: replace { clr c subb a,%1 mov %1,a mov a,%2 subb a,%3 mov %3,a dec %1 cjne %1,#0xff,%4 dec %3 %4: } by { setb c subb a,%1 mov %1,a mov a,%2 subb a,%3 ; Peephole optimized decrement, merged into previous %4: } replace { %1: mov dpl,%2 mov dph,%3 clr a movc a,@a+dptr...
-
A bit more reference on what I was trying to do: uint16_t strlen(register __code const char *s) { __code const char *x = s; while (*x) x++; return x - s; } generates _strlen: mov r6, dpl mov r7, dph mov ar4,r6 mov ar5,r7 00101$: mov dpl,r4 mov dph,r5 clr a movc a,@a+dptr jz 00103$ inc r4 cjne r4,#0x00,00101$ inc r5 sjmp 00101$ 00103$: mov a,r4 clr c subb a,r6 mov r6,a mov a,r5 subb a,r7 mov dpl,r6 mov dph,a ret at first I tried modifying rtrack.c to keep track of value equality, and in genPlusIncr...
-
mcs51: fold extra +1/-1 into genPlus/genMinus
-
But basically the change I made is: Index: sdas/asxxsrc/asmain.c =================================================================== --- sdas/asxxsrc/asmain.c (revision 11952) +++ sdas/asxxsrc/asmain.c (working copy) @@ -1661,7 +1661,7 @@ switch(mp->m_valu) { case O_ASCII: case O_ASCIZ: - if ((d = getnb()) == '\0') + if ((d = getdlm()) == '\0') qerr(); while ((c = getmap(d)) >= 0) outab(c); @@ -1670,7 +1670,7 @@ break; case O_ASCIS: - if ((d = getnb()) == '\0') + if ((d = getdlm()) == '\0') qerr();...
-
If I do that change though there is some weirdness: works: .macro bar arg .ascii arg .endm bar /foo/ also works with the change: .macro bar arg .ascii ^/arg/ .endm bar ^/foo/ doesn't work: .macro bar arg .ascii arg .endm bar ^/foo/ also doesn't work: .macro bar arg .ascii ^/arg/ .endm bar /foo/ it seems like in the macro invocation, for the '^' special case, the delimiters are not passed on into the macro, while without '^' they are...
-
Ohh, I think I get it. The first getnb() should be getdlm() instead (since it tries to get the delimiter character), which has the handling for '^'.
-
(gdb) bt #0 qerr_real () at ../asxxsrc/assubr.c:223 #1 0x000055555555afa7 in getmap (d=94) at ../asxxsrc/aslex.c:422 #2 0x000055555555deb5 in asmbl () at ../asxxsrc/asmain.c:1666 #3 0x000055555555864b in main (argc=2, argv=<optimized out>) at ../asxxsrc/asmain.c:530 Ok, this is where it gets more interesting: case O_ASCIZ: if ((d = getnb()) == '\0') qerr(); while ((c = getmap(d)) >= 0) outab(c); if (mp->m_valu == O_ASCIZ) outab(0); break; Looks like d is the delimiter (indeed 94 matches '^'). I don't...
-
Debugging with #define qerr() do { fprintf(stderr, "qerr: %s:%d\n", __FILE__, __LINE__); qerr_real(); } while (0); shows the error is thrown in aslex.c:422: qerr: ../asxxsrc/aslex.c:422 qerr: ../asxxsrc/aslex.c:422 qerr: ../asxxsrc/aslex.c:422 /tmp/testcase.s:8: Error: <q> missing or improper operators, terminators, or delimiters which is just if ((c=get()) == '\0') qerr(); in "int getmap(int d)"...
-
FWIW asmlnk.txt also documents that there is another way to escape these besides using octal escapes: stringtest ^"should work" But this escape method doesn't work in the current version AFAICS: https://sourceforge.net/p/sdcc/bugs/3150/
-
sdas up-arrow escaped string fails to parse
1
