It is currently Wed Oct 23, 2019 9:47 am

All times are UTC - 7 hours





Post new topic Reply to topic  [ 7 posts ] 
Author Message
PostPosted: Wed Jun 05, 2019 6:24 pm 
Offline
User avatar

Joined: Tue Jun 24, 2008 8:38 pm
Posts: 2318
Location: Fukuoka, Japan
I had this issue a few time and always thought I was doing something wrong but this time I'm pretty sure that something is going on.

I'm using a locally complied version under ubuntu (cc65 V2.17 - Git 535088fe).

Here's the code example:

Code:
// if still active, check Y posisition
if(g_playerBullets.active[m_idx] == TRUE) {
     if(g_playerBullets.y[m_idx]  < 5) {
        g_playerBullets.active[m_idx] = FALSE;
        --g_playerBullets.count;
  }
}


What it does is if the Y value is smaller than 5, do something, pretty basic. The catch: if Y is an 16 bit value, that condition always fails. If I change the Y type to a char, it works fine.

I checked the assembler but I didn't do for a while so I'm a little rusty about the possible cause. here the 16 bit version:

Code:
0002EFr 1               ; if(g_playerBullets.y[m_idx]  < 5) {
0002EFr 1               ;
0002EFr 1                  .dbg   line, "src/manager/bulletsManager.c", 126
0002EFr 1  A2 00           ldx     #$00
0002F1r 1  AD rr rr        lda     _m_idx
0002F4r 1  0A              asl     a
0002F5r 1  90 02           bcc     L0106
0002F7r 1  E8              inx
0002F8r 1  18              clc
0002F9r 1  69 rr        L0106:   adc     #<(_g_playerBullets+6)
0002FBr 1  85 rr           sta     ptr1
0002FDr 1  8A              txa
0002FEr 1  69 rr           adc     #>(_g_playerBullets+6)
000300r 1  85 rr           sta     ptr1+1
000302r 1  A0 01           ldy     #$01
000304r 1  B1 rr           lda     (ptr1),y
000306r 1  AA              tax
000307r 1  88              dey
000308r 1  B1 rr           lda     (ptr1),y
00030Ar 1  C9 05           cmp     #$05
00030Cr 1  8A              txa
00030Dr 1  E9 00           sbc     #$00
00030Fr 1  50 02           bvc     L00BE
000311r 1  49 80           eor     #$80
000313r 1  10 16        L00BE:   bpl     L00B7
000315r 1               ;
000315r 1               ; g_playerBullets.active[m_idx] = FALSE;
000315r 1               ;
000315r 1                  .dbg   line, "src/manager/bulletsManager.c", 127
000315r 1  AC rr rr        ldy     _m_idx
000318r 1  A9 00           lda     #$00
00031Ar 1  99 rr rr        sta     _g_playerBullets+12,y
00031Dr 1               ;
00031Dr 1               ; --g_playerBullets.count;
00031Dr 1               ;
00031Dr 1                  .dbg   line, "src/manager/bulletsManager.c", 128
00031Dr 1  AD rr rr        lda     _g_playerBullets+25
000320r 1  38              sec
000321r 1  E9 01           sbc     #$01
000323r 1  8D rr rr        sta     _g_playerBullets+25
000326r 1  B0 03           bcs     L00B7
000328r 1  CE rr rr        dec     _g_playerBullets+25+1
00032Br 1               ;
00032Br 1               ; if(g_playerBullets.active[m_idx]) {
00032Br 1               ;
00032Br 1                  .dbg   line, "src/manager/bulletsManager.c", 135
00032Br 1  AC rr rr     L00B7:   ldy     _m_idx


My guess is something is going on around line 30F~313, where for some reason, because how the code was generated, it causes the branching to fails with small numbers but I'm too rusty to see it at all but I think I'm close to the cause.

Here's the 8 bit version (Y is now a char):
Code:
0002AFr 1               ; if(g_playerBullets.y[m_idx]  < 5) {
0002AFr 1               ;
0002AFr 1                  .dbg   line, "src/manager/bulletsManager.c", 126
0002AFr 1  AC rr rr        ldy     _m_idx
0002B2r 1  B9 rr rr        lda     _g_playerBullets+6,y
0002B5r 1  C9 05           cmp     #$05
0002B7r 1  B0 16           bcs     L00BB
0002B9r 1               ;
0002B9r 1               ; g_playerBullets.active[m_idx] = FALSE;
0002B9r 1               ;
0002B9r 1                  .dbg   line, "src/manager/bulletsManager.c", 127
0002B9r 1  AC rr rr        ldy     _m_idx
0002BCr 1  A9 00           lda     #$00
0002BEr 1  99 rr rr        sta     _g_playerBullets+9,y
0002C1r 1               ;
0002C1r 1               ; --g_playerBullets.count;
0002C1r 1               ;
0002C1r 1                  .dbg   line, "src/manager/bulletsManager.c", 128
0002C1r 1  AD rr rr        lda     _g_playerBullets+22
0002C4r 1  38              sec
0002C5r 1  E9 01           sbc     #$01
0002C7r 1  8D rr rr        sta     _g_playerBullets+22
0002CAr 1  B0 03           bcs     L00BB
0002CCr 1  CE rr rr        dec     _g_playerBullets+22+1
0002CFr 1               ;
0002CFr 1               ; if(g_playerBullets.active[m_idx]) {
0002CFr 1               ;
0002CFr 1                  .dbg   line, "src/manager/bulletsManager.c", 135
0002CFr 1  AC rr rr     L00BB:   ldy     _m_idx


In that case, there is no need to extract the address in ptr to extract the value since it's 8 bit, the value is loaded in A and proper testing is done, bcs when it's false.

I have been bitten many times by that bug and was sure I was doing something wrong but now I'm getting quite convinced that something is not working as expected, unless I'm doing something in C that I was not supposed to do in cc65? I read the manual, I don't remember anything about testing 16/8 values against each other.

Any information on the subject would be greatly appreciated.


Top
 Profile  
 
PostPosted: Wed Jun 05, 2019 11:14 pm 
Offline
User avatar

Joined: Sun Jan 22, 2012 12:03 pm
Posts: 7608
Location: Canada
The test code in the first case looks correct as far as I can see, but g_playerBullets.y is a signed int (hence the bvc/eor/bpl instead of bcs). Is it possible that you've got negative values in it? What does the debugger show for it when you have a failure?

In contrast char is an unsigned char in CC65, so the second example isn't just 8 bit but also unsigned.

(Also: int is signed int in CC65, and 16-bit.)


Top
 Profile  
 
PostPosted: Thu Jun 06, 2019 12:08 am 
Offline
User avatar

Joined: Tue Jun 24, 2008 8:38 pm
Posts: 2318
Location: Fukuoka, Japan
I'm aware that int is signed and was used for testing borders (if you go out of bound) and that char is always unsigned unless you force it.

As for negative value, it could be possible when going too far up but, like I mentioned, when I set as a char (unsigned) and test when the Y value gets lower than 5, it works right away. The value could have been 48 to make sure it doesn't get negative while testing and it would have failed too.

It's quite a puzzling bug since the C code is simple. My only guess for now is checking the asm so see why it would happens. I will need to trace it too, see how it goes but I need to brush up some of my asm before doing that ^^;;

For now, I just posted it here since I remember there was a more or less similar issue where a constant value failed when tested against and thought it could be a similar bug and wanted to have some opinion on the subject.


Top
 Profile  
 
PostPosted: Thu Jun 06, 2019 11:09 am 
Offline

Joined: Tue Oct 06, 2015 10:16 am
Posts: 975
When in doubt, always test without -O.


Top
 Profile  
 
PostPosted: Thu Jun 06, 2019 2:53 pm 
Offline
User avatar

Joined: Sat Sep 07, 2013 2:59 pm
Posts: 1904
This indeed sounds like the issue I encountered some weeks ago. Comparing an integer against a constant value with the "less than" operator.
viewtopic.php?f=2&t=18843

Do you still get the error when you compile with -O --disable-opt OptCmp3 or -O --disable-opt OptCmp8?

_________________
Available now: My game "City Trouble".
Website: https://megacatstudios.com/products/city-trouble
Trailer: https://youtu.be/IYXpP59qSxA
Gameplay: https://youtu.be/Eee0yurkIW4
German Retro Gamer article: http://i67.tinypic.com/345o108.jpg


Top
 Profile  
 
PostPosted: Thu Jun 06, 2019 4:33 pm 
Offline
User avatar

Joined: Sun Jan 22, 2012 12:03 pm
Posts: 7608
Location: Canada
That problem with OptCmp8 is fixed, at least if you get the latest snapshot. (I'm not aware of a problem with OptCmp3, though it's possible that in your particular case suppressing a valid OptCmp3 optimization suppressed the bad subsequent optimization through OptCmp8?) Even if we're talking about the old version, OptCmp8 shouldn't apply here though, because it's specifically for constant vs. constant compare, not variable vs. constant.

Anyway, don't just start disabling random optimizations. Get some more information before you go down that road:

Just test without -O (on the problem C file only) and see if the problem persists, if no, then we can blame the optimizer. The next step after that is to use -O --debug-opt-output and look over the optimization steps until you find the one that breaks the code.

...but even still, banshaku already posted the generated assembly above. The comparison looks fine, as far as I can tell? The problem should be elsewhere. Could be a cc65 bug, but just as likely is something else.


Top
 Profile  
 
PostPosted: Thu Jun 06, 2019 5:36 pm 
Offline
User avatar

Joined: Tue Jun 24, 2008 8:38 pm
Posts: 2318
Location: Fukuoka, Japan
@calima

Didn't try to remove settings yet since up to now it never was an issue (settings). Will see how it goes!

@DRW

Yes, it does look very similar in behavior to the issue you had in this thread, I do agree. Since I commented exactly about the issue I'm having at the moment in that thread and from the feedback received, it may have been not related, thus not mentioning your bug. I will try those extra options if only -O makes no difference. Better start with smaller tests since I'm not sure what impact those options would have in general ^^;;

@rainwarrior

I'm not using the latest snapshot but I could pull it later then recompiling it locally. as for OptCmd8, it was for constants against constants then. I didn't remember about that specific part, thanks for mentioning it. As for blaming the compiler, I'm used to first blame the dev then after if it still not work, blame the dev since in most case the compiler is not the cause. Still, this time the code is so simple that I have a hard time to continue blaming myself and I had that issue a few time in the past too.

Let see how -O makes it react then. thanks everyone! If it does fix the issue, I guess it will have to be reported to them but for now, let's still blame the dev first :lol:

edit:

Here's the test results:
- without -O -> no difference (I was using -Oi, tried -O and whitout it)
- disabling OptCmp3/OptCmp8 -> no difference

So either I found a new bug or there is something going on in my code that makes it fails. If it was the value itself, it should have failed with an unsigned char but still. For now, I will try to trace the value when defined as an unsigned 16 bits and see if anything strange is going on, I guess. For the time being, I will use unsigned char while researching on that issue since I don't need out of bound checks yet on the Y axis.


Top
 Profile  
 
Display posts from previous:  Sort by  
Post new topic Reply to topic  [ 7 posts ] 

All times are UTC - 7 hours


Who is online

Users browsing this forum: No registered users and 11 guests


You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot post attachments in this forum

Search for:
Jump to:  
Powered by phpBB® Forum Software © phpBB Group