Thread Rating:
  • 0 Vote(s) - 0 Average
  • 1
  • 2
  • 3
  • 4
  • 5
output ASM
#1
Not sure if this is a bug, per se - but it is certainly a bit ugly.

This even survived the peephole optimizer.

Input Basic:

IF myCurrentVector = %1000 then

Output assembly:

ld a, (ix+9)
sub 8
sub 1
jp nc, __LABEL56


Two subs in a row? And worse "Sub 1" instead of a dec?

It later tests:

IF myCurrentVector = %0001 then

as:
ld a, (ix+9)
dec a
sub 1
jp nc, __LABEL63



And I'm not even convinced that's the same test as "equal to x" Surely that's testing "If myCurrentVector>x" anyway?
Reply
#2
I also noticed with:

IF (available = 1) or (available=2) or (available=4) or (available=8)

that the code doesn't do short-circuit testing with OR - it's possible, here to do

if available=1 then stop testing, because we're already true
else if available=2 then stop testing here because....


etc.

Instead it tests all four conditions, all the time. I suspect IF available=1 then elseif might be faster assembly because the short circuit jumps tp the end get put in. However, it would need the action part multiple times, so would be bigger Sad
Reply
#3
britlion Wrote:I also noticed with:

IF (available = 1) or (available=2) or (available=4) or (available=8)

that the code doesn't do short-circuit testing with OR - it's possible, here to do
Short-circut evaluation is not yet implemented, neither is tail recursion (I would like to implement both of them). Of course, it might make your program bigger, but I think it's necessary.
Reply
#4
britlion Wrote:Not sure if this is a bug, per se - but it is certainly a bit ugly.

This even survived the peephole optimizer.

Input Basic:

IF myCurrentVector = %1000 then

Output assembly:

ld a, (ix+9)
sub 8
sub 1
jp nc, __LABEL56

Two subs in a row? And worse "Sub 1" instead of a dec?
Unfortunately, DEC A does not affects carry flag, which is needed later by the JP NC instruction.
See: <!-- m --><a class="postlink" href="http://www.z80.info/z80sflag.htm">http://www.z80.info/z80sflag.htm</a><!-- m -->
I also bumped into this at first. Basically, it needs SUB 8 and test if the result is zero, and gets Carry if the expression (var==0) is TRUE.
If you group these two SUBs into SUB 9, you will get the correct result in A, but will miss some carry information, that might lead to wrong results. Eg. if myCurrentVector is 7, SUB 9 will also raise Carry Flag, which leads to a wrong TRUE results. You have to SUB 8 first, and later SUB 1 (the 1st carry is discarded).

The problem is that here the boolean result is evaluated into A, and then pushed into the stack, popped out, optimized and evaluated. You cant' see this now, because it's been optimized. But the original sequence should be something like:
Code:
ld a, (ix+9)    ; a = myCurrentVector parameter
sub 8             ; a = a - 8
push af          ; stores the tmp result into the stack => (myCurrentVector - 8)
pop af           ; Restore the result. This instruction and the above will be removed
sub 1            ;  Test if A == 0 (Carry if so).
jp nc, __LABEL56 ; If NC => A != 0 => Jump to False condition (ELSE, or END IF)

This code and the 2nd you posted are the same. The obvious optimization is:
Code:
ld a, (ix+9)    ; a = myCurrentVector parameter
sub 8             ; a = a - 8
jp nz, __LABEL56 ; If NC => A != 0 => Jump to False condition (ELSE, or END IF)
But it's not so easy to do that without introducing new -O3 bugs in other places.
I will have a look on it.
Reply
#5
I think I see.

If asked in basic :

IF variable = value then

It does ((variable-value)-1)=overflow.

But surely the most logical thing to do is

variable=value ?

That is

ld a,(variable)
cp value
jp based on zero flag.

I'm aware that sub and cp are in flags terms the same thing, but it would make the code more readable - and you said the same thing, really. The bit I'm stuck on is that the compiler makes the call to do it in two parts in the first place - and you're thinking in terms of the optimizer at -O3 spotting it can shortcut this.

I'm pretty sure a multipass optimizer could do that, by the by - see that sub a follows by sub b could be sub (a+b) - in the same way as push X, pop X can be removed.

But why:

* use SUB at all here? Surely CP is the right instruction, and may well save us having to pull out the variable again (that is, the optimizer might see that we have (ix-9) in A already if we're about to do something to it, which would be common).
* Since the original basic code is testing for equality, why does it end up as a subtract, followed by a test for zero at all? Without referring to anything to do with the optimizer, surely a test-for-constant code block is the parse for if variable=constant ?
Reply
#6
Bump!

I noticed this still happens in latest ZX BASIC version:

britlion Wrote:Input Basic:

IF myCurrentVector = %1000 then

Output assembly:

ld a, (ix+9)
sub 8
sub 1
jp nc, __LABEL56

Notice that "and a / jp nz" works exactly like "sub 1 / jp nc" but it's shorter and faster. Unless you really need flag C to indicate expression result?

Also if you keep track if "sub 8" was the last instruction that affected flags, then your optimizer should be able to remove "and a" in most cases afterwards.
Reply
#7
boriel Wrote:Basically, it needs SUB 8 and test if the result is zero, and gets Carry if the expression (var==0) is TRUE.

Ops! I missed this.

I thought this would be an easy improvement. But if ZX BASIC handles all expression results based on Carry, then my suggestion won't work without more extensive compiler changes...
Reply
#8
boriel Wrote:Short-circut evaluation is not yet implemented, neither is tail recursion (I would like to implement both of them). Of course, it might make your program bigger, but I think it's necessary.

Please keep in mind that introducing short-circut evaluation into an existing language can potentially break lots of already released programs, since it would change program behavior in cases like this:
Code:
IF move(player1) <> 0 OR move(player2) <> 0 THEN
    ...    ' At least one player moved successfully
END IF

In order to avoid breaking existing code, either introduce short-circut evaluation only for purely evaluation expressions (without routine invocations), or make short-circut evaluation optional (disabled by default).

Personally I like short-circut evaluation, but I wouldn't like to find out that some of my released games in ZX BASIC stopped compiling properly when ZX BASIC changed its behavior Smile
Reply


Forum Jump:


Users browsing this thread: 1 Guest(s)