Closed ZornsLemma closed 7 years ago
Having looked at this some more, I see that the optimisation I'm proposing isn't valid, because BRGT/BRLT/BRNE/BREQ only consume one of their two operands. So I don't have any specific reason wishing they used a signed comparison any more. However, I would still be interested to know why the comparison is like it is...
Sorry for the delay in response. But you discovered the difference between the general purpose test and branch vs. the loop optimized versions. I should have done a better job of documenting the differences. As for the comparison, since it was for a bounded loop and assuming the increment/decrement are sane, I felt I could skip the overflow condition. If you can think of a case where that isn't true, I can always put it back in.
Thanks Dave, that makes sense. I couldn't contrive a for plausible loop example which was broken - and moreover I suspect even with an overflow-handling BRLT/BRGT you could still hit problems in corner cases.
I've been looking at adding some more peephole optimisations to the PLASMA compiler - I'll prepare a neat pull request for this at some point in the next week or two - and one thing I hoped to be able to do was to optimise things like "ISGT:BRTRU x" to "BRGT x", "ISLE:BRFLS x" to "BRGT x", etc. This didn't work, I believe because BRGT is not performing a signed comparison.
The BRGT/BRLT code looks a bit like the unsigned comparison at http://www.6502.org/tutorials/compare_beyond.html#4.3, but I note that it checks the N flag afterwards, not the C flag as suggested in that link, so I don't think it is a straightforward unsigned comparison. It also isn't a straightforward signed comparison as it doesn't check V. Is this deliberate, and if so would you mind educating me on why? (It looks to me as though the compiler currently only generates these opcodes for 'for' loops and I do wonder if there's a cunning reason for this implementation as a result.)
If the current behaviour is not deliberate, would you be willing to consider changing this to use a signed comparison instead?