glitches on Apple II

[lazy-schemer]

I built it like this:

make OPTIONS=optsize TARGETS=apple2 dsk

[lazy-schemer]

@oliverschmidt could you give me some advices?

[oliverschmidt]

No good idea :-(

Just some random ideas:

• Maybe the code makes assumptions which aren't met on the emulator used. I'd try the same binary with i.e. AppleWin.
• Maybe cc65 has changed something over time. I'd try to build with a cc65 HEAD around the time the Apple II port was done.

[StewBC]

I can confirm that the issue does indeed now happen. With optsize as decsribed and with optspeed, when drawing the board and it gets to the garbled pieces, the game will crash. I can't look into it right now to make sure this is the issue but I when I look at a version that I previously built, and that does work, it is 24976 bytes. The version I built with a newer compiler (cl65 V2.19 - Git 1093d16 - mabe a month or two ago?) generated an executable of 25961 bytes. I thus suspect a compiler change.

When I have time, I will investigate further (I always kept the previous compiler and just with the last time I updated I didn't - now I sure wish I had ;)

[lazy-schemer]

Hello
I was able to build it with a previous commit of cc65 ( 2a421396, 2019/11/29 )

the reason is still unclear, but it would be ok with me 😀

[oliverschmidt]

• The change in the binary file size is no surprise. If you take a look at the cc65 change log, you'll notice that the compiler basically changes all the time.
• Needless to say that it would be great to bisect this regression. If it is a cc65 problem it would of course be desirable to have a cc65 issue opened. When being able to pinpoint the cc65 change that introduced it, I guess it wouldn't be necessary to additionally create a minimal reproduction program.
• I think for everybody reading this it would be great if a small set of moves were given that trigger the issue.

[StewBC]

From Oliver's list:

1. ACK

2. I did this bisect now - here is the result - The first bad commit:
   commit 8b6d78a075dafde8583c4992d37995d9b4c9db97
   Author: acqn acqn163@outlook.com
   Date: Mon Mar 1 22:11:22 2021 +0800

   Added Opt_a_tosbitwise for 8-bit bitwise operations.

src/cc65/coptstop.c | 83 +++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 83 insertions(+)

But this doesn't make sense to me because I started with head today as bad and as good I used:
commit e5f4ca6b898bf581751901fd5792764adfb7730a
Merge: 128b15a71 1f9594560
Author: Bob Andrews mrdudz@users.noreply.github.com
Date: Wed Apr 19 15:07:16 2023 +0200

Merge pull request #2058 from icepic/patch-1

Update lynxsprite.c

So bad is today and good was Apr 19, 2023 but bisect says first bad commit was 2021? This was my first time using bisect. I suppose that the 2021 commits didn't get merged till after Apr 2023? I don't know how to look to see and my first attempts at finding the asnwers in Google didn't work out - and now I am out of time.

3. Simply running the game and pressing a key on the title screen advances to the board. The bottom (white) pieces stick out above the menu. These pieces can be seen to be corrupt.

[lazy-schemer]

Pieces are broken since PR 2240

OK
bb1b5c36 Bob Andrews mrdudz@users.noreply.github.com on 2023/10/18 at 19:51
OK
Merge pull request #2238 from acqn/BoolOptFix
8e6c0c14 Bob Andrews mrdudz@users.noreply.github.com on 2023/10/26 at 20:06
ERROR
Merge pull request #2240 from acqn/BitwiseOpt
a8d00bfa Bob Andrews mrdudz@users.noreply.github.com on 2023/10/26 at 23:53
ERROR
316ae886 Bob Andrews mrdudz@users.noreply.github.com on 2023/10/26 at 23:56

disks I tested are here.
disk2.zip

[lazy-schemer]

I built them on Intel Mac.

> gcc --version
Apple clang version 15.0.0 (clang-1500.1.0.2.5)
Target: x86_64-apple-darwin22.6.0
Thread model: posix
InstalledDir: /Library/Developer/CommandLineTools/usr/bin

[oliverschmidt]

Can you please check if #12 makes a difference?

[lazy-schemer]

still problem

build with cc65 HEAD 2024/01/09 at 7:02

• cc65-Chess_patch-1_cc65_HEAD_ERR.dsk - broken

build with cc65 8e6c0c14 2023/10/26 at 20:06

• cc65-Chess_patch-1_cc65_8e6c0c14_OK.dsk - OK

cc65's differences between 8e6c0c14 (makes OK build) and a8d00bfa (makes error build)
I hope this can be a hint.

patch-1_builds.zip

[iss000]

The issue appears when mixing XOR and the result from logical operation.
The fix changes from:
inv = blackWhite ^ (piece & PIECE_WHITE) != 0;
to
inv = blackWhite ^ (!!(piece & PIECE_WHITE) != 0);
in src/apple2/platA2.c (line 218)

I'm using latest cc65 build from sources.

EDIT: I didn't checked if other platforms are affected.

[lazy-schemer]

my build also works fine! 😄 thank you !!

[StewBC]

Thank you @iss000 - that does indeed fex the problem. I opted for the line: inv = blackWhite ^ !((piece & PIECE_WHITE) != 0); simply because I would never think to use !!

[oliverschmidt]

Sorry for being slow on the take-up, but isn't (piece & PIECE_WHITE) != 0 already a boolean expression? Can someone please explain what the two changes (the one proposed by @iss000 and the one @StewBC opted for) are doing?

[StewBC]

It's an order of precedence issue. The orifinal code makes inv == 128 and the new code makes it == 1 if piece = 0 and PIECE_WHITE == 128. AT least, I think that's what's going on. Empirically tested I can say that inv == 128 and not 1 in the old vs new code.

[oliverschmidt]

Thanks for the feedback. Sorry for not being precise enough: I'm wondering why it is that way.

[iss000]

Just for the record: my favorites actually are:
inv = blackWhite ^ (piece & PIECE_WHITE ? 1:0);
or
inv = blackWhite ^ ((piece>>7)&1);
Assuming that '!' returns always '0' or '1' isn't always portable.
There are compilers which return '0' or '-1' ... 😄

[oliverschmidt]

@iss000: Am I right to presume that your recent comment is no answer to my question but rather refers to @StewBC's code?

[StewBC]

This code below produces, on MSVC and gcc, the output:
0 0 0 0 0 0 0 0
0 0 0 0 0 0 0 0
0 1 0 1 0 1 0 1
1 0 1 0 1 0 1 0
but on the Apple II it produces:

If I go to an "old working" version of cc65, I get the same output as in MSVC and gcc from cc65. I don't know what the standards say about any of this, but cc65 has changed its behaviour.

This code was used on the Apple II but with cstdio and cprintf, and with a while(!cgetc()){;} at the end of main so I could see the output.

#include <stdio.h>

#define PIECE_WHITE 128
unsigned char gChessBoard[8][8];

void plat_DrawSquare(char position)
{
    char inv;
    char y = position / 8, x = position & 7;
    char piece = gChessBoard[y][x];
    char blackWhite = !((x & 1) ^ (y & 1));

    if(piece) {
        inv = blackWhite ^ (piece & PIECE_WHITE) != 0;
    } else {
        inv = 0;
    }
    if(!(x%8)) {
        printf("\n\r");
    }
    printf("%0d ", inv);
}

void main() {
    int i;
    for(i = 0; i < 16; i++) {
        gChessBoard[i/8][i%8] = 0;
        gChessBoard[6+i/8][i%8] = PIECE_WHITE;
    }
    for(i = 0; i < 16; i++) {
        plat_DrawSquare(i);
    }
    for(i = 0; i < 16; i++) {
        plat_DrawSquare(6*8+i);
    }
}

[oliverschmidt]

Just for the record:

• cgetc() waits for a keyypress so there's no need for a while() loop.
• Building with the default options and starting the resulting program from BASIC makes it return to BASIC so any output stays visible.

The latter implies that there's no need to change the source code of the program above in any way for the Apple II.

[oliverschmidt]

If I go to an "old working" version of cc65, I get the same output as in MSVC and gcc from cc65.

That's the reason why it worked before.

I don't know what the standards say about any of this, but cc65 has changed its behaviour.

Even if the current cc65 behavior is covered by the standard it might still be that the cc65 team isn't aware of the change in behavior and considers it desirable to revert to the former behavior. I personally even assume that they see it this way.

My suggestion would be therefore to further reduce your program to the bare minimum showing the difference to GCC and open a cc65 issue (incl. the info which commit changed the behavior).

[colinleroy]

Hi,
FYI, the regression has been fixed upstream in cc65/cc65#2398, in case you want to remove the workaround commit.

[oliverschmidt]

@colinleroy Thanks for taking care of that! It's actually a nice show case for the usefulness of actually communicating with the cc65 team 😃

[colinleroy]

@colinleroy Thanks for taking care of that! It's actually a nice show case for the usefulness of actually communicating with the cc65 team 😃

And that git bisect is a godsend 😄

[colinleroy]

So bad is today and good was Apr 19, 2023 but bisect says first bad commit was 2021? This was my first time using bisect. I suppose that the 2021 commits didn't get merged till after Apr 2023? I don't know how to look to see and my first attempts at finding the asnwers in Google didn't work out - and now I am out of time.

You suppose correctly, the PR was opened in October 2023 with commits two years old :)

For what it's worth, you can find the history with gitk, searching for the commit hash, it'll mention when it got merged:

[StewBC]

@colinleroy Thank you! I appreciate you sorting this. It still sat in my inbox as something I needed to do when I finished what I am busy doing :)
Now I'll fix the chess commit when I am finished doing what I am busy doing ;)
Anyway, much appreciated!