Isolated PDP-11/20-specific behavior

_posts/2016-11-13-curious-pdp-11-features.md

@@ -34,21 +34,19 @@ And finally, the most interesting bug uncovered by Test 14 involved instructions
 
 	MOV     R0,(R0)+
 
-Imagine that R0 contains 1000.  PDPjs (as well as several other emulators I tested) would write the value 1000 to address
-1000 after auto-incrementing R0 to 1002.
+Imagine that R0 contains 1000.  PDPjs would write the value 1000 to address 1000 after auto-incrementing R0 to
+1002.
 
-Unfortunately, that's wrong.  Apparently, the PDP-11 performs both source and destination address calculations *before*
-reading and writing the source and destination values.  So, in the above example, the value 1002 must be written to address 1000.
-
-I should add that not all emulators get this wrong: [SimH](https://github.com/simh/simh), the gold standard of PDP-11
-emulators, handles it correctly.
+Unfortunately, that's wrong -- for the PDP-11/20 anyway.  Apparently, unlike later (micro-coded) models, the
+PDP-11/20 performs both source and destination address calculations *before* reading and writing the source and
+destination values.  So, in the above example, the value 1002 must be written to address 1000.
 
 Don't confuse this behavior with the order in which the source and destination operands are processed (source operands
 are always decoded first, destination operands next), or with the fact that the auto-increment mode is actually a *post*-
 increment mode, whereas auto-decrement is a *pre*-decrement mode.  Those things are also true, but they have nothing to do
-with this bug.
+with this behavior.
 
-PDPjs resolved the bug by using a special (negative) value to indicate a register source operand, which is then converted
+PDPjs resolved this by using a special (negative) value to indicate a register source operand, which is then converted
 to the register's current value after both operands have been decoded.  Test 14 now passes.
 
 Next up: [Test 15](/apps/pdp11/tapes/diags/#test-15), which uncovered a couple more problems:

apps/pdp11/tapes/diags/README.md

@@ -144,17 +144,12 @@ This test is also noteworthy because it uncovered a bug involving instructions l
 
 	MOV     R0,(R0)+
 
-Imagine that R0 contains 1000.  PDPjs (as well as several other emulators I tested) would write the value 1000
-to address 1000 after auto-incrementing R0 to 1002.
+Imagine that R0 contains 1000.  PDPjs would write the value 1000 to address 1000 after auto-incrementing R0 to
+1002.
 
-Unfortunately, that’s wrong. Apparently, the PDP-11 performs both source and destination address calculations
-before reading and writing the source and destination values.  So, in the above example, the value 1002 must be
-written to address 1000.
-
-I should add that not all emulators get this wrong: [SimH](https://github.com/simh/simh), the gold standard of
-PDP-11 emulators, handles it correctly.  However, it's unclear how much confidence one should place in SimH,
-because as I discovered later when running the [11/70 CPU EXERCISER](#md-11-1170-cpu-exerciser) diagnostic,
-SimH is not infallible.
+Unfortunately, that's wrong -- for the PDP-11/20 anyway.  Apparently, unlike later (micro-coded) models, the
+PDP-11/20 performs both source and destination address calculations *before* reading and writing the source and
+destination values.  So, in the above example, the value 1002 must be written to address 1000.
 
 	MAINDEC-11-D0NA (NEW NUMBER - DAKAA)
 	
@@ -381,7 +376,7 @@ checkStackLimit() handlers based on the CPU model.
 ---
 
 Here's the next test that PDPjs failed.  Again, I found the same sequence of instructions in the 11/40 and 11/45
-version of the diagnostic, at PC 023360 rather than 023450, so I have annotated the code below with DEC's comments:
+version of the diagnostic, at PC 023360 rather than 023450, so I've annotated the code below with DEC's comments:
 
 	023450: 012704 000001          MOV   #1,R4                  ;SET R4
 	023454: 006767 000060          SXT   023540                 ;PRESET DATA=0
@@ -400,30 +395,5 @@ version of the diagnostic, at PC 023360 rather than 023450, so I have annotated
 	023532: 001401                 BEQ   023536
 	023534: 104000                 EMT   000                    ;ERROR: XOR TESTS ABOVE FAILED [They use HLT instead of EMT here]
 
-In my case, the failure was caused by the XOR instruction: it hadn't been updated along with the rest of the
-PDPjs instructions to defer evaluating register operands until BOTH *src* and *dst* operands have been decoded.
-So at this point:
-
-	R0=154343 R1=154112 R2=023422 R3=154501 R4=100000 R5=155066
-	SP=000700 PC=023502 PS=000010 IR=000000 SL=000377 T0 N1 Z0 V0 C0
-	023502: 074767 000032          XOR   PC,023540              ;XOR PC WITH XOR6A (177777)
-
-the XOR instruction was using 23504 for the *src* operand instead of 23506.
-
-Interestingly, this test PASSES in SimH, even though it *also* incorrectly uses 23504 for XOR's *src* operand.
-Why?  Because when SimH gets to this instruction:
-
-	023506: 010767 000030          MOV   PC,023542              ;FORM PC AS USED IN XOR ABOVE
-
-it incorrectly stores 023510 instead of 023512 into memory, and both mistakes end up canceling each other out.
-
-In fact, I'm less impressed with the accuracy of [SimH's](https://github.com/simh/simh) PDP-11 emulation
-than I used to be, because in the process of getting to this particular test within the "11/70 CPU EXERCISER"
-(MAINDEC-11-DEQKC-B1-PB) diagnostic, there were numerous other failures as well (eg, TEST 41 and TEST 45).
-However, SimH is not my baby, so delving into those failures is an exercise for another day.
-
-DISCLAIMER: Until I can run this test on real PDP-11 hardware, I have to acknowledge another possible
-explanation for the above failure: that SimH is *correct*, and that unlike all other registers, the value
-of the PC register is always used immediately, rather than after both *src* and *dst* have been decoded.
-But that strikes me as a strange architectural inconsistency, and given SimH's other "11/70 CPU EXERCISER"
-test failures, I'm going to stick with my original assumption -- for now.
+In my case, the failure was caused by the XOR instruction.  When I fixed the PDP-11/20 issue uncovered by
+[Test 14](#test-14), I had neglected to update XOR, which has some unique operand decoding logic.

modules/pdp11/lib/cpuops.js

@@ -1883,11 +1883,7 @@ PDP11.opWAIT = function(opCode)
 PDP11.opXOR = function(opCode)
 {
     var reg = (opCode >> PDP11.SRCMODE.SHIFT) & PDP11.OPREG.MASK;
-    /*
-     * As per the WARNING in readSrcWord(), we must supply a register number rather than a register value,
-     * which will then be evaluated by updateDstWord() after both the SRC and DST operands have been decoded.
-     */
-    this.updateDstWord(opCode, -reg-1 /* this.regsGen[reg] */, PDP11.fnXOR);
+    this.updateDstWord(opCode, this.regsGen[reg + this.offRegSrc], PDP11.fnXOR);
     this.nStepCycles -= (this.dstMode? (8 + 1) : (2 + 1) + (this.dstReg == 7? 2 : 0));
 };
 

modules/pdp11/lib/cpustate.js

@@ -123,9 +123,21 @@ var Trigger;
  */
 CPUStatePDP11.prototype.initProcessor = function()
 {
+    /*
+     * offRegSrc is a bias added to the register index calculated in readSrcWord() and readSrcByte(),
+     * and by default has no effect on the register index, UNLESS this is a PDP-11/20, in which case the
+     * bias is changed to 8 and we return one of the negative values you see above.  Those negative values
+     * act as signals to writeDstWord() and writeDstByte(), effectively delaying evaluation of the register
+     * until then.
+     */
+    this.offRegSrc = 0;
+    this.maskRegSrcByte = 0xff;
+
     if (this.model == PDP11.MODEL_1120) {
         this.decode = PDP11.op1120.bind(this);
         this.checkStackLimit = this.checkStackLimit1120;
+        this.offRegSrc = 8;
+        this.maskRegSrcByte = -1;
     } else {
         this.decode = PDP11.op1145.bind(this);
         this.checkStackLimit = this.checkStackLimit1145;
@@ -178,8 +190,10 @@ CPUStatePDP11.prototype.initRegs = function()
     this.flagN  = 0x8000;       // PSW N bit
     this.regPSW = 0x000f;       // PSW other bits   (TODO: What's the point of setting the flag bits here, too?)
     this.regsGen = [            // General R0-R7
-        0, 0, 0, 0, 0, 0, 0, this.addrReset
+        0, 0, 0, 0, 0, 0, 0, this.addrReset,
+        -1, -2, -3, -4, -5, -6, -7, -8
     ];
+
     this.regsAlt = [            // Alternate R0-R5
         0, 0, 0, 0, 0, 0
     ];
@@ -2197,10 +2211,10 @@ CPUStatePDP11.prototype.writeWordToPrevSpace = function(opCode, access, data)
 /**
  * readSrcByte(opCode)
  *
- * WARNING: If the SRC operand is a register, we return a negative register number rather than the
- * register value, because the final value of the register must be resolved AFTER the DST operand has
- * been decoded and any pre-decrement or post-increment operations affecting the SRC register have
- * been completed.  See readSrcWord() for more details.
+ * WARNING: If the SRC operand is a register, offRegSrc ensures we return a negative register number
+ * rather than the register value, because on the PDP-11/20, the final value of the register must be
+ * resolved AFTER the DST operand has been decoded and any pre-decrement or post-increment operations
+ * affecting the SRC register have been completed.  See readSrcWord() for more details.
  *
  * @this {CPUStatePDP11}
  * @param {number} opCode
@@ -2213,7 +2227,7 @@ CPUStatePDP11.prototype.readSrcByte = function(opCode)
     var reg = this.srcReg = opCode & PDP11.OPREG.MASK;
     var mode = this.srcMode = (opCode & PDP11.OPMODE.MASK) >> PDP11.OPMODE.SHIFT;
     if (!mode) {
-        result = -reg-1;
+        result = this.regsGen[reg + this.offRegSrc] & this.maskRegSrcByte;
     } else {
         result = this.readByteFromPhysical(this.getAddr(mode, reg, PDP11.ACCESS.READ_BYTE));
     }
@@ -2223,10 +2237,10 @@ CPUStatePDP11.prototype.readSrcByte = function(opCode)
 /**
  * readSrcWord(opCode)
  *
- * WARNING: If the SRC operand is a register, we return a negative register number rather than the
- * register value, because the final value of the register must be resolved AFTER the DST operand has
- * been decoded and any pre-decrement or post-increment operations affecting the SRC register have
- * been completed.
+ * WARNING: If the SRC operand is a register, offRegSrc ensures we return a negative register number
+ * rather than the register value, because on the PDP-11/20, the final value of the register must be
+ * resolved AFTER the DST operand has been decoded and any pre-decrement or post-increment operations
+ * affecting the SRC register have been completed.
  *
  * Here's an example from DEC's "TRAP TEST" (MAINDEC-11-D0NA-PB):
  *
@@ -2255,7 +2269,7 @@ CPUStatePDP11.prototype.readSrcWord = function(opCode)
     var reg = this.srcReg = opCode & PDP11.OPREG.MASK;
     var mode = this.srcMode = (opCode & PDP11.OPMODE.MASK) >> PDP11.OPMODE.SHIFT;
     if (!mode) {
-        result = -reg-1;
+        result = this.regsGen[reg + this.offRegSrc];
     } else {
         result = this.bus.getWord(this.getAddr(mode, reg, PDP11.ACCESS.READ_WORD));
     }