[hatari-devel] DSP stack handling clarification

[Eero Tamminen <oak@xxxxxxxxxxxxxx>]

Hi,

While looking at DSP emulation stack handling in regards to
Sonoluminenz issues, I came up with the attached patch.

It's functionally same as current code, but IMHO makes it
much clearer that the the stack pointer value wraps, and
the check for that.

Laurent, any comments?


	- Eero

diff -r 2ab2e35d5923 src/falcon/dsp_cpu.c
--- a/src/falcon/dsp_cpu.c	Wed Feb 14 19:14:37 2018 +0100
+++ b/src/falcon/dsp_cpu.c	Sun Feb 18 00:38:18 2018 +0200
@@ -1495,23 +1495,25 @@
 static void dsp_stack_push(Uint32 curpc, Uint32 cursr, Uint16 sshOnly)
 {
 	Uint32 stack_error, underflow, stack;
+	bool overflowed;
 
 	stack_error = dsp_core.registers[DSP_REG_SP] & (1<<DSP_SP_SE);
 	underflow = dsp_core.registers[DSP_REG_SP] & (1<<DSP_SP_UF);
 	stack = (dsp_core.registers[DSP_REG_SP] & BITMASK(4)) + 1;
-
-
-	if ((stack_error==0) && (stack & (1<<DSP_SP_SE))) {
-		/* Stack full, raise interrupt */
-		dsp_add_interrupt(DSP_INTER_STACK_ERROR);
-		if (!isDsp_in_disasm_mode)
-			fprintf(stderr,"Dsp: Stack Overflow\n");
-		if (ExceptionDebugMask & EXCEPT_DSP)
+	overflowed = stack & ~BITMASK(4);
+
+	if (unlikely(overflowed)) {
+		stack = 0;	/* wrap */
+		if (!stack_error) {
+			/* Stack full, raise interrupt */
+			dsp_add_interrupt(DSP_INTER_STACK_ERROR);
+			if (!isDsp_in_disasm_mode)
+				fprintf(stderr,"Dsp: Stack Overflow\n");
+			if (ExceptionDebugMask & EXCEPT_DSP)
 			DebugUI(REASON_DSP_EXCEPTION);
-	}
-
-	dsp_core.registers[DSP_REG_SP] = (underflow | stack_error | stack) & BITMASK(6);
-	stack &= BITMASK(4);
+		}
+	}
+	dsp_core.registers[DSP_REG_SP] = (underflow | stack_error | stack);
 
 	if (stack) {
 		/* SSH part */
@@ -1533,22 +1535,26 @@
 static void dsp_stack_pop(Uint32 *newpc, Uint32 *newsr)
 {
 	Uint32 stack_error, underflow, stack;
+	bool underflowed;
 
 	stack_error = dsp_core.registers[DSP_REG_SP] & (1<<DSP_SP_SE);
 	underflow = dsp_core.registers[DSP_REG_SP] & (1<<DSP_SP_UF);
 	stack = (dsp_core.registers[DSP_REG_SP] & BITMASK(4)) - 1;
-
-	if ((stack_error==0) && (stack & (1<<DSP_SP_SE))) {
-		/* Stack empty*/
-		dsp_add_interrupt(DSP_INTER_STACK_ERROR);
-		if (!isDsp_in_disasm_mode)
-			fprintf(stderr,"Dsp: Stack underflow\n");
-		if (ExceptionDebugMask & EXCEPT_DSP)
-			DebugUI(REASON_DSP_EXCEPTION);
-	}
-
-	dsp_core.registers[DSP_REG_SP] = (underflow | stack_error | stack) & BITMASK(6);
-	stack &= BITMASK(4);
+	underflowed = stack & ~BITMASK(4);
+
+	if (unlikely(underflowed)) {
+		stack = BITMASK(4);	/* wrap */
+		if (!stack_error) {
+			/* Stack empty, raise interrupt */
+			dsp_add_interrupt(DSP_INTER_STACK_ERROR);
+			if (!isDsp_in_disasm_mode)
+				fprintf(stderr,"Dsp: Stack Underflow\n");
+			if (ExceptionDebugMask & EXCEPT_DSP)
+				DebugUI(REASON_DSP_EXCEPTION);
+		}
+	}
+
+	dsp_core.registers[DSP_REG_SP] = (underflow | stack_error | stack);
 	*newpc = dsp_core.registers[DSP_REG_SSH];
 	*newsr = dsp_core.registers[DSP_REG_SSL];