PCSX2: Fix stall on branch .. in delay slot #1783

FlatOutPS2 · 2017-01-18T18:18:36Z

Fixes stall when loading a stage in WRC 3.

Removes patch that served as workaround.

Fixes stall when loading a stage in WRC 3. Removes patch that served as workaround.

refractionpcsx2 · 2017-01-19T01:38:34Z

That was quick! I assumed branches in the delay slot would be handled like the vu ones, which would be a pig to fix.

Trying to think if there's any other games that do this too.

Good job

refractionpcsx2 · 2017-01-19T01:42:04Z

Apparently wrc really evolved and possibly midnight club 2 suffered from these too

MonJamp · 2017-01-19T04:04:02Z

@FlatOutPS2 Do you know if this fixes enabling MTVU for Thrillville (you have the game right)?

refractionpcsx2 · 2017-01-19T08:15:05Z

This won't fix games that break when hacks are enabled. The mtvu issue is completely separate

willkuer · 2017-01-19T09:45:09Z

May I ask how you came to this solution? Were you checking what the hack did? I guess there is no forum in the web where people discussed 'well, if you have a stall in branch … delay slot, just put these lines and the errors are gone…' and ps2 documentation also rather seems poor...

refractionpcsx2 · 2017-01-19T09:54:20Z

PS2 documentation is poor and I'm not sure how the EE handles it.

On the VU side, what happens is the VU will take the first branch, read the first instruction, then take the second branch (if it succeeded). Of course if the first branch wasn't taken, as long as it isn't a Likely branch (speaking from the EE side, the VU doesn't have likely's), it would take the second branch only.

however it seems there is an exception for when there is a branch in the branch delay slot, so it's possible the EE does handle it completely differently and just hands it off to the exception manager to deal with. That said we aren't actually setting the BD bit, we're just clearing it?

gregory38 · 2017-01-19T11:13:13Z

I would suggest to run the EE PS2 testsuite. Initial code got an infinite function call loop.
The global variables to handle the flush are not really used. (All registers allocation is completely broken, the big part to redo on x64)

ramapcsx2 · 2017-01-19T14:02:37Z

Woho, an EE recompiler fix. Awesome!
Would be great to test this on a few more games.
The positive (buggy) case will be very clear: It writes "branch xxxx in delay slot!" :)

prafullpcsx2 · 2017-01-19T18:43:03Z

This looks interesting. Will test it thoroughly after my tournament ends in two days time.

pcsx2/x86/ix86-32/iR5900-32.cpp

+						g_cpuFlushedPC = false;
+						g_cpuFlushedCode = false;
+						if (g_maySignalException)
+							xAND(ptr32[&cpuRegs.CP0.n.Cause], ~(1 << 31)); // BD


ssakash · 2017-01-20T06:43:57Z

That said we aren't actually setting the BD bit, we're just clearing it?

Yeah, doesn't make sense to me either. According to the sources, we only need to set BD when encountering exceptions other than the reset, NMI, performance counter and debug on the delay slot. I don't seem to find any information about clearing the bit at any cases.

Also seems like @sudonim1 was the one who introduced it in d4bc1a6, so assigning this to him as he probably knows how this works.

gregory38 · 2017-01-20T09:53:02Z

@ssakash here the theory
When you encounter an exception/interrupt, you need to stop the program to handle the exception/interrupt. When it is done you must resume the execution. Therefore you must save the PC before jumping to the exception handler (saved in the EPC register)
For example:

0x400:load v0, 0;
0x404:load v1, v0; // may generate a TLB miss
0x408:load v0, v0;

2nd load will generate a TLB miss. So you save the PC (here 0x404) in the EPC register. You execute the TLB miss handler. Then you return to the program with a jump $EPC.

Now imagine, that exception occurred in a delay slot.

0x400:load v0, 0;
0x404:jump 0x410;
0x408:load v1, v0; // may generate a TLB miss
0x40C:load v0, 1;
0x410: print $v0;

Again the CPU will save the PC of the instruction that raise the exception. Here 0x408. Handler will be executed. And finally jump $EPC (reminder 0x408). So next instruction will be 0x40C and 0x410. As you can see you didn't resume the execution correctly (will print 1). An exception/interrupt must never change the program behavior. So instead you save $EPC + the BD flag. This way, you will jump back to $EPC-4 (so 0x404) so program will resume correctly (will print 0).

There is 2 level of exceptions, hence BD/BD2. It is easier to handle in the CPU (and potentially it allow lvl2 exception in lvl1 exception). LVL2 exception must never happen in the emulation.

Now remain to answer the behavior of what must happen if there is a branch in the delay slot case. I don't remember the exact behavior. Either 2nd jump is a NOP or final address is the one of the 2nd jump. Behavior can be checked with the testsuite.

load v0, imm;
jump 0x1000;
jump 0x2000; // jump in delay slot (could trigger a tlb miss)
jump 0x3000;
print PC; <= likely 0x1000 (or 0x2000)

BD will be cleared when you resume program execution (check ERET opcode). Potentially the behavior is equivalent to (for every cycle)

if (cpu is in std mode)
  bd = jump/branch opcode ? 1 : 0;
else if (cpu is in exception 1 mode)
  bd2 = jump/branch opcode ? 1 : 0;
else
   nop

gregory38 · 2017-01-20T10:09:14Z

@FlatOutPS2
I forgot to say in my wall of text, could you create an handy function to avoid copy/past.

ssakash · 2017-01-20T10:13:33Z

@gregory38

Thanks for the write up, really appreciated. :)

I forgot to say in my wall of text, could you create an handy function to avoid copy/past.

Agreed, I was also planning on mentioning it but forgot about it for some reasons.

sudonim1 · 2017-01-21T18:35:41Z

I think that commit of mine was just preparation for future work that has only kind of happened because at least at the time it was impossible for any recompiler instruction to raise an exception.

gregory38 · 2017-01-26T16:14:23Z

Please add a common function. And note to self, I need to run the test suite.

gregory38 · 2017-01-30T17:51:04Z

do you have the instructions around the branch in branch delay slot.

From the HW testsuite:


	// This is technically not valid MIPS anyway.
	// But it seems to always take target 2?
	asm volatile (
		".set    noreorder\n"

		"li      $t0, 0\n"
		"beq     $t0, $t0, target1_%=\n"
		"beq     $t0, $t0, target2_%=\n"
		"beq     $t0, $t0, target3_%=\n"
		"beq     $t0, $t0, target4_%=\n"
		"nop\n"

		"target1_%=:\n"
		"li      %0, 1\n"
		"j       skip_%=\n"
		"nop\n"

		"target2_%=:\n"
		"li      %0, 2\n"
		"j       skip_%=\n"
		"nop\n"

		"target3_%=:\n"
		"li      %0, 3\n"
		"j       skip_%=\n"
		"nop\n"

		"target4_%=:\n"
		"li      %0, 4\n"
		"j       skip_%=\n"
		"nop\n"

		"skip_%=:\n"

		: "+r"(result)
	);

	printf("beq: delay branch: %08x\n", result);

So far we return 1 (even with this PR), and the HW 2.

sudonim1 · 2017-01-31T22:11:46Z

So basically branch in branch delay slot on real hardware works like you'd expect. The delay slot is fetched/the first branch executed, the first branch target fetched/the second branch executed, the first branch target executed/the second branch target fetched, execution continues from the second branch target.

…

On 30 January 2017 at 17:51, Gregory Hainaut ***@***.***> wrote: do you have the instructions around the branch in branch delay slot. From the HW testsuite: // This is technically not valid MIPS anyway. // But it seems to always take target 2? asm volatile ( ".set noreorder\n" "li $t0, 0\n" "beq $t0, $t0, target1_%=\n" "beq $t0, $t0, target2_%=\n" "beq $t0, $t0, target3_%=\n" "beq $t0, $t0, target4_%=\n" "nop\n" "target1_%=:\n" "li %0, 1\n" "j skip_%=\n" "nop\n" "target2_%=:\n" "li %0, 2\n" "j skip_%=\n" "nop\n" "target3_%=:\n" "li %0, 3\n" "j skip_%=\n" "nop\n" "target4_%=:\n" "li %0, 4\n" "j skip_%=\n" "nop\n" "skip_%=:\n" : "+r"(result) ); printf("beq: delay branch: %08x\n", result); So far we return 1 (even with this PR), and the HW 2. — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#1783 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AGvnmTx9eTNemv7ve3kmBD6PB54M94_Nks5rXiMKgaJpZM4LnOF8> .

gregory38 · 2017-02-18T11:03:22Z

@FlatOutPS2 could you create a generic function ?

FlatOutPS2 · 2017-02-18T18:55:37Z

could you create a generic function ?

I already did, but I was still to look into some possibly related issues before updating the PR. I just haven't gotten around to it yet.

ghost · 2017-09-15T19:15:12Z

Any status?

MrCK1 · 2017-09-15T20:07:46Z

I don't think this will be merged anytime soon. Flatout was in charge and well, he's gone for now.

ghost · 2018-01-15T20:44:08Z

Apparently Midnight club games are fixed with this PR.

MrCK1 · 2018-01-15T20:45:04Z

They all work fine, not sure what you mean?

FlatOutPS2 · 2018-01-15T20:51:11Z

Someone once said != actually true.

I don't see any post on the forum about testing this PR with that game.

FlatOutPS2 · 2018-01-15T21:35:22Z

Anyway is there any other games affected by this?

No. The only other mentions of this issue are due to cheats/hacks.

refractionpcsx2 · 2018-01-15T21:41:51Z

The only other game I know I mentioned earlier in the thread is WRC 4 i think it is

FlatOutPS2 · 2018-01-15T21:47:09Z

WRC Evolved. ;) Though it should also affect WRC 4, but that has other issues not related to this PR.

ghost · 2019-04-06T08:45:55Z

I tested this PR with a lot of games and I have 2 improvement:

-Action replay MAX
-WRC 2005 avec Sébastien Loeb

I think it's safe to merge as I did not notice any regression with it.

lightningterror · 2019-04-07T22:09:12Z

@refractionpcsx2 , @ramapcsx2 what are your thoughts, what are we gonna do with this?
@atomic83GitHub seems to have tested a bunch of games with no new issues popping up.

refractionpcsx2 · 2019-04-07T23:38:58Z

If it works, merge it. Better than what we've got. however due tot he gamedb conflicts you might have to open a new PR

ramapcsx2 · 2019-04-08T12:26:36Z

@lightningterror
If you're doing a new PR, could you add a comment somewhere that links to the earlier discussion here?
It seems this code change works in favor of the games, but is still not entirely correct.
@gregory38 tested this in 2017 using the ps2autotests suite and remarked "So far we return 1 (even with this PR), and the HW 2."

I feel this bit shouldn't be lost. A comment could either link here, or summarize that there's still problems with the test suite.
Thanks :)

Edit: Link to that specific test:
https://github.com/unknownbrackets/ps2autotests/blob/master/tests/cpu/ee/branchdelay.cpp

Fixes stall when loading a stage in WRC 3. Original pr #1783 Collaborator: lightningterror

lightningterror · 2019-04-08T19:42:21Z

Closing in favor of #2919

Fixes stall when loading a stage in WRC 3. Original pr #1783 Collaborator: lightningterror

PCSX2: Fix stall on branch .. in delay slot

fb5c5a3

Fixes stall when loading a stage in WRC 3. Removes patch that served as workaround.

FlatOutPS2 added Core Recompiler labels Jan 18, 2017

ssakash assigned refractionpcsx2 Jan 19, 2017

ssakash reviewed Jan 20, 2017

View reviewed changes

pcsx2/x86/ix86-32/iR5900-32.cpp

g_cpuFlushedPC = false;

g_cpuFlushedCode = false;

if (g_maySignalException)

xAND(ptr32[&cpuRegs.CP0.n.Cause], ~(1 << 31)); // BD

This comment was marked as spam.

Sign in to view

ssakash assigned sudonim1 Jan 20, 2017

lightningterror added the Postponed label Nov 19, 2017

ghost mentioned this pull request Jan 19, 2018

Mafia - City of Lost Heaven (TLB misses) #1496

Open

ghost mentioned this pull request Mar 2, 2018

GameDB: Patch update and EETimingHack fix for Drakan 2 demo and Ruff Trigger #2332

Merged

lightningterror pushed a commit that referenced this pull request Apr 8, 2019

pcsx2: Fix stall on branch .. in delay slot.

6389c1b

Fixes stall when loading a stage in WRC 3. Original pr #1783 Collaborator: lightningterror

lightningterror mentioned this pull request Apr 8, 2019

PCSX2: Fix stall on branch in delay slot #2919

Merged

lightningterror closed this Apr 8, 2019

lightningterror pushed a commit that referenced this pull request Apr 9, 2019

pcsx2: Fix stall on branch .. in delay slot.

135d93b

Fixes stall when loading a stage in WRC 3. Original pr #1783 Collaborator: lightningterror

lightningterror pushed a commit that referenced this pull request Apr 10, 2019

pcsx2: Fix stall on branch .. in delay slot.

17ac536

Fixes stall when loading a stage in WRC 3. Original pr #1783 Collaborator: lightningterror

someother1n mentioned this pull request Jul 23, 2020

WRC 3 black screen on boot #3559

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PCSX2: Fix stall on branch .. in delay slot #1783

PCSX2: Fix stall on branch .. in delay slot #1783

FlatOutPS2 commented Jan 18, 2017

refractionpcsx2 commented Jan 19, 2017

refractionpcsx2 commented Jan 19, 2017

MonJamp commented Jan 19, 2017

refractionpcsx2 commented Jan 19, 2017

willkuer commented Jan 19, 2017

refractionpcsx2 commented Jan 19, 2017 •

edited

Loading

gregory38 commented Jan 19, 2017

ramapcsx2 commented Jan 19, 2017 •

edited

Loading

prafullpcsx2 commented Jan 19, 2017

This comment was marked as spam.

ssakash commented Jan 20, 2017

gregory38 commented Jan 20, 2017

gregory38 commented Jan 20, 2017

ssakash commented Jan 20, 2017

sudonim1 commented Jan 21, 2017

gregory38 commented Jan 26, 2017 •

edited

Loading

gregory38 commented Jan 30, 2017

sudonim1 commented Jan 31, 2017 via email

gregory38 commented Feb 18, 2017

FlatOutPS2 commented Feb 18, 2017

ghost commented Sep 15, 2017

MrCK1 commented Sep 15, 2017 •

edited

Loading

ghost commented Jan 15, 2018

MrCK1 commented Jan 15, 2018

FlatOutPS2 commented Jan 15, 2018

FlatOutPS2 commented Jan 15, 2018

refractionpcsx2 commented Jan 15, 2018

FlatOutPS2 commented Jan 15, 2018 •

edited

Loading

ghost commented Apr 6, 2019

lightningterror commented Apr 7, 2019

refractionpcsx2 commented Apr 7, 2019

ramapcsx2 commented Apr 8, 2019 •

edited

Loading

lightningterror commented Apr 8, 2019

PCSX2: Fix stall on branch .. in delay slot #1783

PCSX2: Fix stall on branch .. in delay slot #1783

Conversation

FlatOutPS2 commented Jan 18, 2017

refractionpcsx2 commented Jan 19, 2017

refractionpcsx2 commented Jan 19, 2017

MonJamp commented Jan 19, 2017

refractionpcsx2 commented Jan 19, 2017

willkuer commented Jan 19, 2017

refractionpcsx2 commented Jan 19, 2017 • edited Loading

gregory38 commented Jan 19, 2017

ramapcsx2 commented Jan 19, 2017 • edited Loading

prafullpcsx2 commented Jan 19, 2017

This comment was marked as spam.

ssakash commented Jan 20, 2017

gregory38 commented Jan 20, 2017

gregory38 commented Jan 20, 2017

ssakash commented Jan 20, 2017

sudonim1 commented Jan 21, 2017

gregory38 commented Jan 26, 2017 • edited Loading

gregory38 commented Jan 30, 2017

sudonim1 commented Jan 31, 2017 via email

gregory38 commented Feb 18, 2017

FlatOutPS2 commented Feb 18, 2017

ghost commented Sep 15, 2017

MrCK1 commented Sep 15, 2017 • edited Loading

ghost commented Jan 15, 2018

MrCK1 commented Jan 15, 2018

FlatOutPS2 commented Jan 15, 2018

FlatOutPS2 commented Jan 15, 2018

refractionpcsx2 commented Jan 15, 2018

FlatOutPS2 commented Jan 15, 2018 • edited Loading

ghost commented Apr 6, 2019

lightningterror commented Apr 7, 2019

refractionpcsx2 commented Apr 7, 2019

ramapcsx2 commented Apr 8, 2019 • edited Loading

lightningterror commented Apr 8, 2019

refractionpcsx2 commented Jan 19, 2017 •

edited

Loading

ramapcsx2 commented Jan 19, 2017 •

edited

Loading

gregory38 commented Jan 26, 2017 •

edited

Loading

MrCK1 commented Sep 15, 2017 •

edited

Loading

FlatOutPS2 commented Jan 15, 2018 •

edited

Loading

ramapcsx2 commented Apr 8, 2019 •

edited

Loading