Merge pull request #8 from fer-ppj/fix-0-comparisons

Fix unsigned comparisons with zero
fer-ppj · Jan 16, 2019 · c1a9eb8 · c1a9eb8
2 parents 7cd6cda + 65a549e
commit c1a9eb8
Show file tree

Hide file tree

Showing 4 changed files with 140 additions and 3 deletions.
diff --git a/consoleapp/friscsim.js b/consoleapp/friscsim.js
@@ -552,12 +552,25 @@ var FRISC = function() {
     },
 
     // Leave 'dest' undefined to only get the side-effect of changing the flags.
+    // dest <- src1 - src2 + carry
     _SUB_internal: function(src1, src2, carry, dest) {
+      var op2 = (typeof src2==='number') ? src2 : this._r[src2];
       // Do a three-way add with two's complements of src2 and with the carry bit.
       this._ADD_three(this._r[src1],
-          twosComplement(typeof src2==='number' ? src2 : this._r[src2], this._WORD_BITS),
+          twosComplement(op2, this._WORD_BITS),
           carry,
           dest);
+      if (op2 === 0) {
+        // Doing two's complement on 0 creates a carry on the MSB (the one's
+        // complement is all 1s). This is particularly important for the
+        // implementation of CMP when the second operand is 0. For example,
+        // comparing 1 with 0 must generate a carry so that the _UGE condition
+        // is recognized.
+        //
+        // TODO: Check the exact flag semantics of SBC in FRISC when the input
+        // carry is 1.
+        this._setFlag(this._f.C, 1);
+      }
     },
 
     _i: {

diff --git a/lib/friscjs-browser.js b/lib/friscjs-browser.js
diff --git a/src/friscsim.js b/src/friscsim.js
@@ -552,12 +552,25 @@ var FRISC = function() {
     },
 
     // Leave 'dest' undefined to only get the side-effect of changing the flags.
+    // dest <- src1 - src2 + carry
     _SUB_internal: function(src1, src2, carry, dest) {
+      var op2 = (typeof src2==='number') ? src2 : this._r[src2];
       // Do a three-way add with two's complements of src2 and with the carry bit.
       this._ADD_three(this._r[src1],
-          twosComplement(typeof src2==='number' ? src2 : this._r[src2], this._WORD_BITS),
+          twosComplement(op2, this._WORD_BITS),
           carry,
           dest);
+      if (op2 === 0) {
+        // Doing two's complement on 0 creates a carry on the MSB (the one's
+        // complement is all 1s). This is particularly important for the
+        // implementation of CMP when the second operand is 0. For example,
+        // comparing 1 with 0 must generate a carry so that the _UGE condition
+        // is recognized.
+        //
+        // TODO: Check the exact flag semantics of SBC in FRISC when the input
+        // carry is 1.
+        this._setFlag(this._f.C, 1);
+      }
     },
 
     _i: {

diff --git a/tests/e2e-tests.js b/tests/e2e-tests.js
@@ -183,6 +183,104 @@ var tests = [
   new T.Test("3 > -1", function() {
     testCondition("SGT", 3, -1, true);
   }),
+
+
+  ///////////////////////////////
+  // Unsigned comparisons with 0.
+  ///////////////////////////////
+
+  // ULT
+  new T.Test("-1 < 0", function() {
+    testCondition("ULT", -1, 0, false);
+  }),
+  new T.Test("0 < 0", function() {
+    testCondition("ULT", 0, 0, false);
+  }),
+  new T.Test("1 < 0", function() {
+    testCondition("ULT", 1, 0, false);
+  }),
+
+  // ULE
+  new T.Test("-1 <= 0", function() {
+    testCondition("ULE", -1, 0, false);
+  }),
+  new T.Test("0 <= 0", function() {
+    testCondition("ULE", 0, 0, true);
+  }),
+  new T.Test("1 <= 0", function() {
+    testCondition("ULE", 1, 0, false);
+  }),
+
+  // UGE
+  new T.Test("-1 >= 0", function() {
+    testCondition("UGE", -1, 0, true);
+  }),
+  new T.Test("0 >= 0", function() {
+    testCondition("UGE", 0, 0, true);
+  }),
+  new T.Test("1 >= 0", function() {
+    testCondition("UGE", 1, 0, true);
+  }),
+
+  // UGT
+  new T.Test("-1 > 0", function() {
+    testCondition("UGT", -1, 0, true);
+  }),
+  new T.Test("0 > 0", function() {
+    testCondition("UGT", 0, 0, false);
+  }),
+  new T.Test("1 > 0", function() {
+    testCondition("UGT", 1, 0, true);
+  }),
+
+
+  /////////////////////////////
+  // Signed comparisons with 0.
+  /////////////////////////////
+
+  // SLT
+  new T.Test("-1 < 0", function() {
+    testCondition("SLT", -1, 0, true);
+  }),
+  new T.Test("0 < 0", function() {
+    testCondition("SLT", 0, 0, false);
+  }),
+  new T.Test("1 < 0", function() {
+    testCondition("SLT", 1, 0, false);
+  }),
+
+  // SLE
+  new T.Test("-1 <= 0", function() {
+    testCondition("SLE", -1, 0, true);
+  }),
+  new T.Test("0 <= 0", function() {
+    testCondition("SLE", 0, 0, true);
+  }),
+  new T.Test("1 <= 0", function() {
+    testCondition("SLE", 1, 0, false);
+  }),
+
+  // SGE
+  new T.Test("-1 >= 0", function() {
+    testCondition("SGE", -1, 0, false);
+  }),
+  new T.Test("0 >= 0", function() {
+    testCondition("SGE", 0, 0, true);
+  }),
+  new T.Test("1 >= 0", function() {
+    testCondition("SGE", 1, 0, true);
+  }),
+
+  // SGT
+  new T.Test("-1 > 0", function() {
+    testCondition("SGT", -1, 0, false);
+  }),
+  new T.Test("0 > 0", function() {
+    testCondition("SGT", 0, 0, false);
+  }),
+  new T.Test("1 > 0", function() {
+    testCondition("SGT", 1, 0, true);
+  }),
 ];
 
 module.exports.stats = T.runTests(tests, {