Jit64: dcbf + dcbi #2663

degasus · 2015-06-26T11:40:12Z

Both dcbx instructions are called quite often and usually NOPs. So inline to first check within the JIT itself to skip over them much faster.

Source/Core/Core/PowerPC/Jit64/Jit_LoadStore.cpp

+	ABI_PushRegistersAndAdjustStack({}, 0);
+	MOV(32, R(ABI_PARAM1), R(addr));
+	MOV(32, R(ABI_PARAM2), Imm32(32));
+	MOV(32, R(ABI_PARAM3), Imm32(0));


JMC47 · 2015-06-26T13:31:04Z

Rogue Squadron 3 crashes with idleskipping off with this pull request.

degasus · 2015-06-26T14:44:23Z

@JMC47 sounds like a synchronization issue. This shouldn't affect idle skipping. Does it also crash on single core?

JMC47 · 2015-06-26T20:41:28Z

@degasus it doesn't crash with just "sync on idle skipping" disabled. It only crashes with full idle skipping disabled.

Not a sync issue.

Source/Core/Core/PowerPC/JitCommon/JitCache.h

@@ -67,7 +68,6 @@ class ValidBlockBitSet final
 	};
 	std::unique_ptr<u32[]> m_valid_block;


degasus · 2015-08-06T18:16:36Z

@JMC47 Is Rogue Squadron 3 fixed now?

JMC47 · 2015-08-06T18:23:58Z

Yeah, it's fixed.

JMC47 · 2015-08-06T18:38:20Z

Seems to randomly crash when playing various games now, though.

JMC47 · 2015-08-07T15:40:26Z

Summer Games II (Virtual Console) crashes on this PR but not on master.

Source/Core/Core/PowerPC/Jit64/Jit_LoadStore.cpp

+	// dcbi
+	if (inst.SUBOP10 == 470)
+	{
+		TEST(16, M(&DSP::g_dspState), Imm16(1 << 9));


degasus · 2015-08-08T16:38:05Z

flacs> i like it

JMC47 · 2015-08-08T19:07:45Z

Summer Games II fixed, now Super Mario 64 is crashing.

degasus · 2015-08-23T11:04:37Z

I've dropped the ICBI commit, seems like I was wrong with the icache.
@JMC47 Are there still some broken games?

JMC47 · 2015-08-24T10:10:38Z

Super Mario 64 no longer crashing.

Source/Core/Core/HW/DSP.cpp

@@ -145,7 +145,7 @@ struct ARAMInfo

 // STATE_TO_SAVE
 static ARAMInfo g_ARAM;
-static UDSPControl g_dspState;
+UDSPControl g_dspState;


- dynamically allocate third scratch register instead of forcing ECX - use LEA as 3 operand add if possible - use BT,JC instead of SHR,TEST,JNZ - merge MOV,TEST - use appropriate ABI function (no asm change)

degasus · 2015-08-24T16:55:04Z

btw, I think it's worth to also implement this in another way, so a very common code seems to be:

dcbi r0, r3
addi r3, r3, 32
bdnz+

dcbf r0, r3
addi r3, r3, 32
bdnz+

dcbst r0, r3
addi r3, r3, 32
bdnz+

dcbz r0, r3
addi r3, r3, 32
bdnz+

dcbt r0, r3
dcbst r0, r3
addi r3, r3, 32
bdnz+

degasus · 2015-08-24T17:54:00Z

ready to merge

Jit64: dcbf + dcbi

PR dolphin-emu#2663 added a Jit64 implementation of dcbX and a fast path to skip JIT cache invalidation. Unfortunately, there is a mismatch between address spaces in this optimization. It tests the effective address (with the top 3 bits cleared) against the valid block bitset which is intended to be indexed by physical address. While this works in the common case, it fails (for example) when the effective address is in the 7E... region (a.k.a. "fake VMEM"). This may also fall apart under more complex memory mapping scenarios requiring full MMU emulation. The good news is that even without this fast path, the underlying call to JitInterface::InvalidateICache() still does very little work in the common case. It correctly translates the effective address to a physical address which it tests against the valid block bitset, skipping invalidation if it is not necessary. As such, the cost of removing the fast path should not be too high. The Jit64 implementation is retained, though all it does now is emit a call. This may be marginally more efficient than simple interpreter fallback, which involves an extra call. The JitArm64 implementation is removed, as I am not set up for testing that platform. I don't expect this to make a meaningful difference. The game Happy Feet is fixed by this change, as it loads code in the 7E... address region and depends upon JIT cache invalidation in reponse to dcbf. https://bugs.dolphin-emu.org/issues/12133

…ath to skip JIT cache invalidation. Unfortunately, there is a mismatch between address spaces in this optimization. It tests the effective address (with the top 3 bits cleared) against the valid block bitset which is intended to be indexed by physical address. While this works in the common case, it fails (for example) when the effective address is in the 7E... region (a.k.a. "fake VMEM"). This may also fall apart under more complex memory mapping scenarios requiring full MMU emulation. The good news is that even without this fast path, the underlying call to JitInterface::InvalidateICache() still does very little work in the common case. It correctly translates the effective address to a physical address which it tests against the valid block bitset, skipping invalidation if it is not necessary. As such, the cost of removing the fast path should not be too high. The Jit64 implementation is retained, though all it does now is emit a call. This may be marginally more efficient than simple interpreter fallback, which involves an extra call. The JitArm64 implementation is removed, as I am not set up for testing that platform. I don't expect this to make a meaningful difference. The game Happy Feet is fixed by this change, as it loads code in the 7E... address region and depends upon JIT cache invalidation in reponse to dcbf. https://bugs.dolphin-emu.org/issues/12133

PR dolphin-emu#2663 added a Jit64 implementation of dcbX and a fast path to skip JIT cache invalidation. Unfortunately, there is a mismatch between address spaces in this optimization. It tests the effective address (with the top 3 bits cleared) against the valid block bitset which is intended to be indexed by physical address. While this works in the common case, it fails (for example) when the effective address is in the 7E... region (a.k.a. "fake VMEM"). This may also fall apart under more complex memory mapping scenarios requiring full MMU emulation. The good news is that even without this fast path, the underlying call to JitInterface::InvalidateICache() still does very little work in the common case. It correctly translates the effective address to a physical address which it tests against the valid block bitset, skipping invalidation if it is not necessary. As such, the cost of removing the fast path should not be too high. The Jit64 implementation is retained, though all it does now is emit a call. This may be marginally more efficient than simple interpreter fallback, which involves an extra call. The JitArm64 implementation is removed, as I am not set up for testing that platform. I don't expect this to make a meaningful difference. The game Happy Feet is fixed by this change, as it loads code in the 7E... address region and depends upon JIT cache invalidation in reponse to dcbf. https://bugs.dolphin-emu.org/issues/12133

PR dolphin-emu#2663 added a Jit64 implementation of dcbX and a fast path to skip JIT cache invalidation. Unfortunately, there is a mismatch between address spaces in this optimization. It tests the effective address (with the top 3 bits cleared) against the valid block bitset which is intended to be indexed by physical address. While this works in the common case, it fails (for example) when the effective address is in the 7E... region (a.k.a. "fake VMEM"). This may also fall apart under more complex memory mapping scenarios requiring full MMU emulation. The good news is that even without this fast path, the underlying call to JitInterface::InvalidateICache() still does very little work in the common case. It correctly translates the effective address to a physical address which it tests against the valid block bitset, skipping invalidation if it is not necessary. As such, the cost of removing the fast path should not be too high. The Jit64 implementation is retained, though all it does now is emit a call. This is marginally more efficient than simple interpreter fallback, which involves an extra call. The JitArm64 implementation has also been fixed. The game Happy Feet is fixed by this change, as it loads code in the 7E... address region and depends upon JIT cache invalidation in reponse to dcbf. https://bugs.dolphin-emu.org/issues/12133

degasus added the WIP / do not merge Work in progress (do not merge) label Jun 26, 2015

degasus force-pushed the dcbx branch from cf1928b to 03a68c7 Compare June 26, 2015 11:43

Sintendo reviewed Jun 26, 2015
View reviewed changes

Source/Core/Core/PowerPC/Jit64/Jit_LoadStore.cpp

ABI_PushRegistersAndAdjustStack({}, 0);

MOV(32, R(ABI_PARAM1), R(addr));

MOV(32, R(ABI_PARAM2), Imm32(32));

MOV(32, R(ABI_PARAM3), Imm32(0));

This comment was marked as off-topic.

Sign in to view

degasus force-pushed the dcbx branch from 03a68c7 to 1791b47 Compare June 26, 2015 14:45

lioncash reviewed Jun 28, 2015
View reviewed changes

degasus force-pushed the dcbx branch from 1791b47 to c7049ae Compare August 6, 2015 18:04

degasus force-pushed the dcbx branch from 93c970b to 020c31c Compare August 6, 2015 23:20

degasus force-pushed the dcbx branch from 020c31c to 7a111cb Compare August 8, 2015 13:27

degasus changed the title ~~Jit64: dcbf + dcbi~~ Jit64: dcbf + dcbi + icbi Aug 8, 2015

delroth reviewed Aug 8, 2015
View reviewed changes

Source/Core/Core/PowerPC/Jit64/Jit_LoadStore.cpp

// dcbi

if (inst.SUBOP10 == 470)

{

TEST(16, M(&DSP::g_dspState), Imm16(1 << 9));

This comment was marked as off-topic.

Sign in to view

degasus removed the WIP / do not merge Work in progress (do not merge) label Aug 8, 2015

degasus force-pushed the dcbx branch from d7515d0 to eae94cb Compare August 23, 2015 10:56

degasus changed the title ~~Jit64: dcbf + dcbi + icbi~~ Jit64: dcbf + dcbi Aug 23, 2015

lioncash reviewed Aug 24, 2015
View reviewed changes

Source/Core/Core/HW/DSP.cpp

@@ -145,7 +145,7 @@ struct ARAMInfo

// STATE_TO_SAVE

static ARAMInfo g_ARAM;

static UDSPControl g_dspState;

UDSPControl g_dspState;

This comment was marked as off-topic.

Sign in to view

degasus and others added 3 commits August 24, 2015 18:33

Jit64: implement dcbf + dcbi

6f34b27

Jit64: some cache flush changes

ac84d6d

- dynamically allocate third scratch register instead of forcing ECX - use LEA as 3 operand add if possible - use BT,JC instead of SHR,TEST,JNZ - merge MOV,TEST - use appropriate ABI function (no asm change)

Jit64: Optimize dcbx

0d92c8f

degasus force-pushed the dcbx branch from eae94cb to 0d92c8f Compare August 24, 2015 17:50

degasus added a commit that referenced this pull request Aug 25, 2015

Merge pull request #2663 from degasus/dcbx

24cb650

Jit64: dcbf + dcbi

degasus merged commit 24cb650 into dolphin-emu:master Aug 25, 2015

degasus deleted the dcbx branch February 3, 2019 22:17

smurf3tte mentioned this pull request Dec 6, 2020

Jit: Fix correctness issue in dcbf/dcbi/dcbst #9314

Merged

Jit64: dcbf + dcbi #2663

Jit64: dcbf + dcbi #2663

degasus commented Jun 26, 2015

This comment was marked as off-topic.

JMC47 commented Jun 26, 2015

degasus commented Jun 26, 2015

JMC47 commented Jun 26, 2015

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

degasus commented Aug 6, 2015

JMC47 commented Aug 6, 2015

JMC47 commented Aug 6, 2015

JMC47 commented Aug 7, 2015

This comment was marked as off-topic.

degasus commented Aug 8, 2015

JMC47 commented Aug 8, 2015

degasus commented Aug 23, 2015

JMC47 commented Aug 24, 2015

This comment was marked as off-topic.

degasus commented Aug 24, 2015

degasus commented Aug 24, 2015

		@@ -67,7 +68,6 @@ class ValidBlockBitSet final
		};
		std::unique_ptr<u32[]> m_valid_block;

Jit64: dcbf + dcbi #2663

Jit64: dcbf + dcbi #2663

Conversation

degasus commented Jun 26, 2015

This comment was marked as off-topic.

JMC47 commented Jun 26, 2015

degasus commented Jun 26, 2015

JMC47 commented Jun 26, 2015

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

degasus commented Aug 6, 2015

JMC47 commented Aug 6, 2015

JMC47 commented Aug 6, 2015

JMC47 commented Aug 7, 2015

This comment was marked as off-topic.

degasus commented Aug 8, 2015

JMC47 commented Aug 8, 2015

degasus commented Aug 23, 2015

JMC47 commented Aug 24, 2015

This comment was marked as off-topic.

degasus commented Aug 24, 2015

degasus commented Aug 24, 2015