Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix 8/16-bit ADC/SBC #3601

Merged
merged 4 commits into from
Apr 30, 2024
Merged

Conversation

alyssarosenzweig
Copy link
Collaborator

alternative to #3600

@Sonicadvance1
Copy link
Member

Interestingly, while this solves the unittest, it doesn't solve the rendering issue that FF7 is encountering.

@alyssarosenzweig
Copy link
Collaborator Author

Interestingly, while this solves the unittest, it doesn't solve the rendering issue that FF7 is encountering.

Curious. Guess we're going to want a second unit test too, lest we optimize this later 😅

Racking my brain for what behaviour change could possibly happen here.

@Sonicadvance1
Copy link
Member

%ifdef CONFIG
{
  "RegData": {
    "RAX": "0xedededee26260e6c",
    "RBX": "0x121212129498c16d"
  }
}
%endif

; FEX had a bug with smaller than 32-bit operations corrupting sbb and adc results.
; A small test that tests both sbb and adc to ensure it returns data correctly.
; This was noticed in Final Fantasy 7 (steamid 39140) having broken rendering on the title screen.
mov rax, 0x4142434445464748
mov rbx, 0x5152535455565758
mov rcx, 0x6162636465666768

clc
sbb al, bl
sbb ax, bx
sbb eax, ebx
sbb rax, rbx

%assign i 0
%rep 256
sbb al, [rel .data1 + i]
%assign i i+1
%endrep

%assign i 0
%rep 256
sbb ax, [rel .data2 + i]
%assign i i+1
%endrep

%assign i 0
%rep 256
sbb eax, [rel .data4 + i]
%assign i i+1
%endrep


%assign i 0
%rep 256
sbb rax, [rel .data8 + i]
%assign i i+1
%endrep

stc
%assign i 0
%rep 256
sbb al, [rel .data1 + i]
%assign i i+1
%endrep

%assign i 0
%rep 256
sbb ax, [rel .data2 + i]
%assign i i+1
%endrep

%assign i 0
%rep 256
sbb eax, [rel .data4 + i]
%assign i i+1
%endrep


%assign i 0
%rep 256
sbb rax, [rel .data8 + i]
%assign i i+1
%endrep




clc
adc bl, cl
adc bx, cx
adc ebx, ecx
adc rbx, rcx


%assign i 0
%rep 256
adc bl, [rel .data1 + i]
%assign i i+1
%endrep

%assign i 0
%rep 256
adc bx, [rel .data2 + i]
%assign i i+1
%endrep

%assign i 0
%rep 256
adc ebx, [rel .data4 + i]
%assign i i+1
%endrep


%assign i 0
%rep 256
adc rbx, [rel .data8 + i]
%assign i i+1
%endrep


stc
%assign i 0
%rep 256
adc bl, [rel .data1 + i]
%assign i i+1
%endrep

%assign i 0
%rep 256
adc bx, [rel .data2 + i]
%assign i i+1
%endrep

%assign i 0
%rep 256
adc ebx, [rel .data4 + i]
%assign i i+1
%endrep


%assign i 0
%rep 256
adc rbx, [rel .data8 + i]
%assign i i+1
%endrep




hlt

.data1:
%assign i 0
%rep 256
db i
%assign i i+1
%endrep

.data2:
%assign i 0
%rep 256
dw i
%assign i i+1
%endrep

.data4:
%assign i 0
%rep 256
dd i
%assign i i+1
%endrep

.data8:
%assign i 0
%rep 256
dq i
%assign i i+1
%endrep

I have a very absurd test that fails but I have yet to be able to reduce the test case to something reasonable.

@alyssarosenzweig
Copy link
Collaborator Author

Hey, if it's good enough for me to repro, that'll do! thanks 😸

@alyssarosenzweig
Copy link
Collaborator Author

Oh, derp, I know what this is. Fix coming. thanks!

alyssarosenzweig and others added 4 commits April 29, 2024 20:00
Reproduces broken rendering in Final Fantasy 7 (SteamID 39140)
Signed-off-by: Alyssa Rosenzweig <[email protected]>
@Sonicadvance1
Copy link
Member

With the latest changes FF7 now renders correctly again!

Copy link
Member

@Sonicadvance1 Sonicadvance1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice little optimization and nice bug fix!

@Sonicadvance1 Sonicadvance1 merged commit 9c6f749 into FEX-Emu:main Apr 30, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants