It seems the problem is that sdcc thinks &psd[j] is an invariant, and calculates it before the loop, even though j changes in the loop. I wonder, why the other ports are not affected.
Philipp
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Ok, it's not j, it's j << 1. Nevertheless, before CSE the sitatuion looks the same for z80 and hc08. But for z80 the error is introduced by CSE (not GCSE). Even though AFAIK there is no port-specific stuff in CSE.
Philipp
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
The CSE code invokes (*port->cseOk)(ic, pdic) to see if the current backend is happy with the potential replacement, if port->cseOk is non-nullL. For z80, the function pointer is null, so CSE assumes z80 is happy with all potential replacements found. For hc08, there is a defined function and it is currently rejecting all proposed CSE replacements. For mcs51, there is a defined function and it accepts some replacements and rejects others.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Shouldn't we change that to 'reject all' when there is no cseOk function? If a backend wants to accept everything I prefer it to be explicit about that instead of assuming it's ok.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
I consider the current behaviour to be OK: CSE should do replacements that are
1) correct
2) considered to make the code better in some way in the opinion of whoever wrote some part of CSE.
AFAIK, the cseOk-mechanism is for having a port-specific cvariant of 2). Z80 currently doesn't have a CSE cost estimation, so maybe it gets some cse that might increase code size a bit for some code.
Disabling the CSE optimizatiomn this tracker item is about in the z80 backend would be just a workaround for a CSE bug.
Philipp
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Yes, this is entirely a CSE bug; I did not mean to imply otherwise. I was only intending to point out how CSE becomes port specific and why this bug emerges for some and not others.
I believe the default is to "accept all" when there is no cseOk function because that was the original behavior before that callback was added to the port structure (revision 1548 by sandeep, Sat Nov 10 06:35:39 2001 UTC)
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
It seems the problem is that sdcc thinks &psd[j] is an invariant, and calculates it before the loop, even though j changes in the loop. I wonder, why the other ports are not affected.
Philipp
It is not even %psd[j]. sdcc plainly keeps another copy of j, that is never incremented and uses it in the loop to calculate &psd[j].
Philipp
Ok, it's not j, it's j << 1. Nevertheless, before CSE the sitatuion looks the same for z80 and hc08. But for z80 the error is introduced by CSE (not GCSE). Even though AFAIK there is no port-specific stuff in CSE.
Philipp
Smaller example that reproduces the bug(see line 22).
The CSE code invokes (*port->cseOk)(ic, pdic) to see if the current backend is happy with the potential replacement, if port->cseOk is non-nullL. For z80, the function pointer is null, so CSE assumes z80 is happy with all potential replacements found. For hc08, there is a defined function and it is currently rejecting all proposed CSE replacements. For mcs51, there is a defined function and it accepts some replacements and rejects others.
Shouldn't we change that to 'reject all' when there is no cseOk function? If a backend wants to accept everything I prefer it to be explicit about that instead of assuming it's ok.
I consider the current behaviour to be OK: CSE should do replacements that are
1) correct
2) considered to make the code better in some way in the opinion of whoever wrote some part of CSE.
AFAIK, the cseOk-mechanism is for having a port-specific cvariant of 2). Z80 currently doesn't have a CSE cost estimation, so maybe it gets some cse that might increase code size a bit for some code.
Disabling the CSE optimizatiomn this tracker item is about in the z80 backend would be just a workaround for a CSE bug.
Philipp
Yes, this is entirely a CSE bug; I did not mean to imply otherwise. I was only intending to point out how CSE becomes port specific and why this bug emerges for some and not others.
I believe the default is to "accept all" when there is no cseOk function because that was the original behavior before that callback was added to the port structure (revision 1548 by sandeep, Sat Nov 10 06:35:39 2001 UTC)
What's with this item? Was it fixed or not? And if it was fixed then why wasn't this closed? Was it a reminder to add a regression test later on?
AFAIK it is not yet fixed.
Philipp
Ok, never mind. We don't even have a "Group" anymore. This is probably an artifact from the changeover of the SF site 2 years ago.
Fix for bug [#2495] in [r9700] also fixed this bug.
Related
Bugs:
#2495Last edit: Maarten Brock 2017-10-17