Menu
▾
▴
[sdcc-devel] [ sdcc-Patches-887161 ] add->inc optimisation
|
From: SourceForge.net <no...@so...> - 2006-10-01 11:01:57
|
Patches item #887161, was opened at 2004-01-29 19:12 Message generated for change (Comment added) made by maartenbrock You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=300599&aid=887161&group_id=599 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: None Group: None Status: Closed Resolution: Accepted Priority: 5 Submitted By: Stas Sergeev (stsp) Assigned to: Maarten Brock (maartenbrock) Summary: add->inc optimisation Initial Comment: Hi. The attached patch is a peephole rule that reduces add to inc. It saves 9 bytes for my program - not too much, but perhaps can be usefull for someone (provided the patch is correct of course:) ---------------------------------------------------------------------- >Comment By: Maarten Brock (maartenbrock) Date: 2006-10-01 13:01 Message: Logged In: YES user_id=888171 Stas, I understand it already was forgotten. We all would have if it hadn't been for the tracker system. To answer your question: SDCC would become very slow. IIRC someone investigated this once and found SDCC spends most of it's time in the peephole optimizer. Maybe we could make it an option. This is also the reason I removed those unused rules. They do no harm but do take time to parse. Maarten ---------------------------------------------------------------------- Comment By: Stas Sergeev (stsp) Date: 2006-10-01 12:46 Message: Logged In: YES user_id=501371 Hi Marten, thanks for taking a look. This all have already swapped out of my head, but have you found an answer to my question here why the peephole iteration is not restarted after any peephole succeeded? I don't know if it is still relevant, but when I tested that, a hack to always restart the peephole gave me a smaller code. ---------------------------------------------------------------------- Comment By: Maarten Brock (maartenbrock) Date: 2006-09-29 14:04 Message: Logged In: YES user_id=888171 I've implemented the optimization in genPlusIncr() and genMinusDec() in gen.c. I've disabled rules 104, 207, 209 & 212. All in SDCC 2.6.1 #4390. ---------------------------------------------------------------------- Comment By: Maarten Brock (maartenbrock) Date: 2006-09-27 18:01 Message: Logged In: YES user_id=888171 I just had a look at this patch and related stuff. 1) Stas, your peephole rule 104.b seems to have little resemblance to 104(.a). Besides it is already implemented as rule 214. 2) Peephole rule 104 doesn't seem to get triggered anymore. It's not used in the regression tests. Nor are 207, 209 and 212. Bernhard are these the rules you referred to? 3) The patch for genPlusIncr and genMinusDec looks ok. I'm running regression tests now. If all goes well, I'll apply the patch and remove the 4 unused rules. Maarten ---------------------------------------------------------------------- Comment By: Stas Sergeev (stsp) Date: 2004-02-02 18:50 Message: Logged In: YES user_id=501371 Hi guys. I was not aware that such a simple patch can be so controversial:) Well, at least I can try to make it better. > But the patch seems to trigger a bug I can only hope someone is up to fix it:) > add a,#0x02 <-- OK (quicker than inc a, inc a) Yes I see. My next patch does exactly this by adding a proper peephole rule. It no longer puts 2 incs. > I already had a look at this optimization long time ago. > The problem was that it broke a quite optimistic peephole, Please take a look into a new patch. Although it may still break the peephole you refer to, it adds another peephole and overall saves 25 bytes for me (which is quite significant after all). Actually 11 out of that 25 bytes are saved because I added a "restart" for 236.a (otherwise my new rule doesn't activate). Which makes me to wonder: why the every rule is not restarting the iteration by default? I hacked SDCCpeeph.c to make it always restart, and got even a smaller code. So why they are not restarting by default? Or, perhaps even better, why peephole doesn't do multiple iterations over the whole rule list? ---------------------------------------------------------------------- Comment By: Bernhard Held (bernhardheld) Date: 2004-02-01 14:49 Message: Logged In: YES user_id=203539 I already had a look at this optimization long time ago. The problem was that it broke a quite optimistic peephole, which is needed for bp calculation in --stack-auto. Fixing this peephole resulted in a much larger code. Therefore I didn't commit the stuff. Please run the regression tests (especially test-mcs51-stack-auto) before comitting anything. I'll have a look at my archive tomorrow. ---------------------------------------------------------------------- Comment By: Frieder Ferlemann (frief) Date: 2004-02-01 12:54 Message: Logged In: YES user_id=589052 Hi, yes, that looks obviously correct. But the patch seems to trigger a bug for variable c (a,b and d(!) are fine) in the following example: xdata unsigned char a,b,c,d; tst() { a++; b--; c+=2; d-=2; } ;plusinc.c:7: c+=2; ; genAssign mov dptr,#_c movx a,@dptr ; genPlus ; genPlusIncr ; Peephole 177.d removed redundant move mov r2,a mov dptr,#_c add a,#0x02 <-- OK (quicker than inc a, inc a) inc a <-- ? movx @dptr,a Someone else should have a look... Frieder ---------------------------------------------------------------------- Comment By: Stas Sergeev (stsp) Date: 2004-01-31 16:47 Message: Logged In: YES user_id=501371 Thanks for reviewing, the peephole opt is a tricky business indeed. That sequence comes from genPlus, and is unlikely can be prepended or appended with something for peephole (looks self-contained). Perhaps the better idea would be to patch the code generator itself then? I've hardcoded that optimization in gen.c. Now it is guaranteed to work only with 8bit operands and, as a bonus, also replaces the addition of 2 by two increments. For consistency I also added this to genMinusDec, but I don't have a test-case for that, so no idea how it works. Maybe that new patch is more viable than the previous one? ---------------------------------------------------------------------- Comment By: Frieder Ferlemann (frief) Date: 2004-01-31 11:18 Message: Logged In: YES user_id=589052 Hi Stas, unfortunately "inc" does not change the flags register (neither Z nor C so f.e. a following "addc" could fail). This makes this peephole potentialy unsafe for 16/32 bit instructions, so I feel uneasy to add it. Maybe you can prepend or append instructions (like the mov a,_bp in peephole 212) to make this peephole more specific. - Frieder ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=300599&aid=887161&group_id=599 |