From a28c52c2508f64101f56cad056c44ebeda63249b Mon Sep 17 00:00:00 2001
From: Thomas Harte <thomas.harte@gmail.com>
Date: Sat, 4 Jan 2020 22:22:33 -0500
Subject: [PATCH] Fixes A7-relative JSRs.

I completely withdraw my earlier statement re: the test cases.
---
 .../xcshareddata/xcschemes/Clock Signal.xcscheme          | 4 ++--
 .../Clock SignalTests/68000 Comparative Tests/readme.md   | 5 +----
 OSBindings/Mac/Clock SignalTests/68000ComparativeTests.mm | 2 +-
 Processors/68000/Implementation/68000Storage.cpp          | 8 ++++++--
 4 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/OSBindings/Mac/Clock Signal.xcodeproj/xcshareddata/xcschemes/Clock Signal.xcscheme b/OSBindings/Mac/Clock Signal.xcodeproj/xcshareddata/xcschemes/Clock Signal.xcscheme
index 7b1a6914b..2a1c34006 100644
--- a/OSBindings/Mac/Clock Signal.xcodeproj/xcshareddata/xcschemes/Clock Signal.xcscheme	
+++ b/OSBindings/Mac/Clock Signal.xcodeproj/xcshareddata/xcschemes/Clock Signal.xcscheme	
@@ -68,8 +68,8 @@
    </TestAction>
    <LaunchAction
       buildConfiguration = "Release"
-      selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB"
-      selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB"
+      selectedDebuggerIdentifier = ""
+      selectedLauncherIdentifier = "Xcode.IDEFoundation.Launcher.PosixSpawn"
       enableASanStackUseAfterReturn = "YES"
       disableMainThreadChecker = "YES"
       launchStyle = "0"
diff --git a/OSBindings/Mac/Clock SignalTests/68000 Comparative Tests/readme.md b/OSBindings/Mac/Clock SignalTests/68000 Comparative Tests/readme.md
index 430da440a..7399e5a7e 100644
--- a/OSBindings/Mac/Clock SignalTests/68000 Comparative Tests/readme.md	
+++ b/OSBindings/Mac/Clock SignalTests/68000 Comparative Tests/readme.md	
@@ -32,9 +32,6 @@ So the output is very scattergun approach, with a lot of redundancy.
 
 ## Questionable Results
 
-I am presently unconvinced by the results for:
-
-* the N flag on many of the [A/S/N]BCD results, as these often seem to conflict with Bart Trzynadlowski's 68knotes.txt; and
-* the alleged results of A7-relative JSR, for which the emulator I was testing against appears to have computed a target address for the value of A7 after pushing the existing PC to the stack, which is unlikely because the first of that instruction's program fetches occurs before any stack activity.  
+I am presently unconvinced by the results for  the N flag on many of the [A/S/N]BCD results, as these often seem to conflict with Bart Trzynadlowski's 68knotes.txt.
 
 This emulator seems not yet to generate values for the undocumented flags of DIVU and DIVS that match those listed here, but that's through lack of documentation. The objective here is to test my implementation of behaviour I am able to find descriptions of against other people's implementations of that same behaviour, to flush out errors in my comprehension and implementation.
diff --git a/OSBindings/Mac/Clock SignalTests/68000ComparativeTests.mm b/OSBindings/Mac/Clock SignalTests/68000ComparativeTests.mm
index 1a32b19e5..38aa41718 100644
--- a/OSBindings/Mac/Clock SignalTests/68000ComparativeTests.mm	
+++ b/OSBindings/Mac/Clock SignalTests/68000ComparativeTests.mm	
@@ -27,7 +27,7 @@
 
 - (void)setUp {
 	// To limit tests run to a subset of files and/or of tests, uncomment and fill in below.
-//	_fileSet = [NSSet setWithArray:@[@"move_tofrom_srccr.json"]];
+//	_fileSet = [NSSet setWithArray:@[@"jmp_jsr.json"]];
 //	_testSet = [NSSet setWithArray:@[@"CHK 41a8"]];
 }
 
diff --git a/Processors/68000/Implementation/68000Storage.cpp b/Processors/68000/Implementation/68000Storage.cpp
index 29c84ee8f..1bbd45d7f 100644
--- a/Processors/68000/Implementation/68000Storage.cpp
+++ b/Processors/68000/Implementation/68000Storage.cpp
@@ -2371,6 +2371,10 @@ struct ProcessorStorageConstructor {
 							// ... but otherwise assume that the true source of a destination will be the computed source address.
 							program.set_source(storage_, &storage_.effective_address_[0]);
 
+							// Beware below: PrepareJSR will pre-emptively subtract four from A7 in order
+							// to facilitate the peculiar stack write order of JSR. Therefore any address
+							// calculation that might be a function of A7 needs to be done before PrepareJSR.
+
 							const int mode = combined_mode(ea_mode, ea_register);
 							switch(mode) {
 								default: continue;
@@ -2391,15 +2395,15 @@ struct ProcessorStorageConstructor {
 								case XXXw:		// JSR (xxx).W
 								case d16PC:		// JSR (d16, PC)
 								case d16An:		// JSR (d16, An)
-									op(Action::PrepareJSR);
 									op(address_action_for_mode(mode) | MicroOp::SourceMask);
+									op(Action::PrepareJSR);
 									op(Action::PerformOperation, seq("n np nW+ nw np", { ea(1), ea(1) }));
 								break;
 
 								case d8PCXn:	// JSR (d8, PC, Xn)
 								case d8AnXn:	// JSR (d8, An, Xn)
-									op(Action::PrepareJSR);
 									op(calc_action_for_mode(mode) | MicroOp::SourceMask);
+									op(Action::PrepareJSR);
 									op(Action::PerformOperation, seq("n nn np nW+ nw np", { ea(1), ea(1) }));
 								break;
 							}