Make IMULD more compatible in overflow cases

This commit is contained in:
Jeff Parsons 2015-10-17 08:39:38 -07:00
commit a3107a73ef
6 changed files with 159 additions and 76 deletions

View file

@ -175,7 +175,7 @@ function Debugger(parmsDbg)
* aVariables is an object with properties that grows as setVariable() assigns more variables;
* each property corresponds to one variable, where the property name is the variable name (ie,
* a string beginning with a letter or underscore, followed by zero or more additional letters,
* digits, or underscores) and the property value is the variable's numeric value. See doSet()
* digits, or underscores) and the property value is the variable's numeric value. See doVar()
* and setVariable() for details.
*
* Note that parseValue(), through its reliance on str.parseInt(), assumes a default base of 16
@ -284,22 +284,22 @@ if (DEBUGGER) {
'g [#]': "go [to #]",
'h': "halt",
'i [#]': "input port #",
'if': "eval expression",
'k': "stack trace",
'l': "load sector(s)",
"ln": "list nearest symbol(s)",
'm': "messages",
'mouse': "mouse action", // syntax: mouse {action} {delta} (eg, mouse x 10, mouse click 0, etc)
'o [#]': "output port #",
'p': "step over", // other variations: pr (step and dump registers)
'print': "print expression",
'r': "dump/set registers",
'reset': "reset machine",
't [#]': "trace", // other variations: tr (trace and dump registers)
'u [#]': "unassemble",
'x': "execution options",
'if': "eval expression",
"ln": "list nearest symbol(s)",
'mouse': "mouse action", // syntax: mouse {action} {delta} (eg, mouse x 10, mouse click 0, etc)
'print': "print expression",
'reset': "reset machine",
'set': "assign expression",
'ver': "display version"
'v': "print version",
'var': "assign variable"
};
/*
@ -5150,6 +5150,18 @@ if (DEBUGGER) {
/**
* evalExpression(aVals, aOps, cOps)
*
* In Node, if you set a variable to 0x80000001; ie:
*
* foo=0x80000001|0
*
* and then calculate foo*foo using "(foo*foo).toString(2)", the result is:
*
* '11111111111111111111111111111100000000000000000000000000000000'
*
* which is slightly incorrect because it has overflowed JavaScript's floating-point precision.
*
* 0x80000001 in decimal is -2147483647, so the product is 4611686014132420609, which is 0x3FFFFFFF00000001.
*
* @this {Debugger}
* @param {Array.<number>} aVals
* @param {Array.<string>} aOps
@ -5397,7 +5409,7 @@ if (DEBUGGER) {
var fDefined = false;
if (value !== undefined) {
fDefined = true;
sValue = str.toHexLong(value) + " (" + value + ')'; /* + str.toBinBytes(value) */
sValue = str.toHexLong(value) + " (" + value + '=' + str.toBinBytes(value) + ')';
}
sVar = (sVar != null? (sVar + ": ") : "");
this.println(sVar + sValue);
@ -6388,20 +6400,20 @@ if (DEBUGGER) {
};
/**
* doSet(sCmd)
* doVar(sCmd)
*
* The command must be of the form "{variable} = [{expression}]", where expression may contain constants,
* operators, registers, symbols, other variables, or nothing at all; in the latter case, the variable, if
* any, is deleted.
*
* Other supported shorthand: "set" with no parameters prints the values of all variables, and "set {variable}"
* Other supported shorthand: "var" with no parameters prints the values of all variables, and "var {variable}"
* prints the value of the specified variable.
*
* @this {Debugger}
* @param {string} sCmd
* @return {boolean} true if valid "set" assignment, false if not
* @return {boolean} true if valid "var" assignment, false if not
*/
Debugger.prototype.doSet = function(sCmd)
Debugger.prototype.doVar = function(sCmd)
{
var a = sCmd.match(/^\s*([A-Z_]?[A-Z0-9_]*)\s*(=?)\s*(.*)$/i);
if (a) {
@ -7675,13 +7687,6 @@ if (DEBUGGER) {
}
this.doRegisters(asArgs);
break;
case 's':
if (asArgs[0] == "set") {
if (!this.doSet(sCmd.substr(3))) {
result = false;
}
}
break;
case 't':
this.doTrace(asArgs[0], asArgs[1]);
break;
@ -7689,6 +7694,12 @@ if (DEBUGGER) {
this.doUnassemble(asArgs[1], asArgs[2], 8);
break;
case 'v':
if (asArgs[0] == "var") {
if (!this.doVar(sCmd.substr(3))) {
result = false;
}
break;
}
this.println((APPNAME || "PCjs") + " version " + (XMLVERSION || APPVERSION) + " (" + this.cpu.model + (COMPILED? ",RELEASE" : (DEBUG? ",DEBUG" : ",NODEBUG")) + (PREFETCH? ",PREFETCH" : ",NOPREFETCH") + (TYPEDARRAYS? ",TYPEDARRAYS" : (FATARRAYS? ",FATARRAYS" : ",LONGARRAYS")) + (BACKTRACK? ",BACKTRACK" : ",NOBACKTRACK") + ')');
break;
case 'x':

View file

@ -1140,9 +1140,6 @@ X86.fnIMUL8 = function(dst, src)
* Example 3: 16 * -8 (0xf8) = -128 (0xff80): carry is clear (the sign bit *still* fits in the lower 8 bits)
* Example 4: 16 * -16 (0xf0) = -256 (0xff00): carry is set (the sign bit no longer fits in the lower 8 bits)
*
* An earlier version of this function assumed it simply needed to check bit 7 of the result to determine carry,
* which was completely broken.
*
* @this {X86CPU}
* @param {number} dst
* @param {number} src (null; AL is the implied src)
@ -1195,6 +1192,7 @@ X86.fnIMULn = function(dst, src)
{
var fOverflow, result;
dst = this.getIPWord();
if (this.sizeData == 2) {
result = (((src << 16) >> 16) * ((dst << 16) >> 16))|0;
fOverflow = (result > 32767 || result < -32768);
@ -1227,13 +1225,6 @@ X86.fnIMULn = function(dst, src)
*
* This sets regMDHi:regMDLo to the 64-bit result of dst * src, both of which are treated as signed.
*
* TODO: Some potential optimizations include:
*
* 1) Early outs if either parameter is zero, since the result will obviously be zero
* 2) Using "normal" JavaScript multiplication if both parameters are >= -32768 && <= 32767
*
* Refer to: http://stackoverflow.com/questions/13597364/32-bit-signed-multiplication-with-a-64-bit-result-in-javascript
*
* @this {X86CPU}
* @param {number} dst (any 32-bit number, treated as signed)
* @param {number} src (any 32-bit number, treated as signed)
@ -1271,9 +1262,6 @@ X86.fnIMUL32 = function(dst, src)
* Example 3: 256 * -128 (0xff80) = -32768 (0xffff8000): carry is clear (the sign bit *still* fits in the lower 16 bits)
* Example 4: 256 * -256 (0xff00) = -65536 (0xffff0000): carry is set (the sign bit no longer fits in the lower 16 bits)
*
* An earlier version of this function assumed it simply needed to check bit 15 of the result to determine carry,
* which was completely broken.
*
* @this {X86CPU}
* @param {number} dst
* @param {number} src (null; AX or EAX is the implied src)
@ -1321,6 +1309,9 @@ X86.fnIMULw = function(dst, src)
/**
* fnIMULrw(dst, src)
*
* This function exists for 16-bit IMUL instructions that produce a 16-bit result instead of a 32-bit result
* (and don't implicitly use the accumulator).
*
* @this {X86CPU}
* @param {number} dst
* @param {number} src
@ -1328,6 +1319,10 @@ X86.fnIMULw = function(dst, src)
*/
X86.fnIMULrw = function(dst, src)
{
/*
* Unlike fnIMULrd() below, we can use normal JavaScript multiplication, because there's no danger of
* overflowing the floating-point result and losing accuracy in the bottom 16 bits.
*/
var result = (((dst << 16) >> 16) * ((src << 16) >> 16))|0;
if (result > 32767 || result < -32768) {
this.setCF(); this.setOF();
@ -1342,6 +1337,9 @@ X86.fnIMULrw = function(dst, src)
/**
* fnIMULrd(dst, src)
*
* This function exists for 32-bit IMUL instructions that produce a 32-bit result instead of a 64-bit result
* (and don't implicitly use the accumulator).
*
* @this {X86CPU}
* @param {number} dst
* @param {number} src
@ -1349,15 +1347,32 @@ X86.fnIMULrw = function(dst, src)
*/
X86.fnIMULrd = function(dst, src)
{
var result = dst * src;
if (result > 2147483647 || result < -2147483648) {
/*
* The following code works, but I've stopped using it because it produces different results from an actual CPU
* when overflow occurs; the bottom 32 bits of the result are still supposed to be accurate.
*
* And unfortunately, we cannot achieve that level of compatibility using normal JavaScript multiplication,
* because the result may be too large to fit in a JavaScript floating-point variable, which means we could lose
* accuracy in the bottom 32 bits, which would defeat what we're trying to achieve here. So we must use the
* slower fnIMUL32() function.
*
* var result = dst * src;
* if (result > 2147483647 || result < -2147483648) {
* this.setCF(); this.setOF();
* } else {
* this.clearCF(); this.clearOF();
* }
* result |= 0;
*/
X86.fnIMUL32.call(this, dst, src);
var fOverflow = (this.regMDHi != (this.regMDLo >> 31));
if (fOverflow) {
this.setCF(); this.setOF();
} else {
this.clearCF(); this.clearOF();
}
result |= 0;
this.nStepCycles -= (this.regEA === X86.ADDR_INVALID? 9 : 12);
return result;
return this.regMDLo;
};
/**
@ -2055,19 +2070,19 @@ X86.fnMULb = function(dst, src)
*
* This sets regMDHi:regMDLo to the 64-bit result of dst * src, both of which are treated as unsigned.
*
* TODO: Some potential optimizations include:
*
* 1) Early outs if either parameter is zero, since the result will obviously be zero
* 2) Using "normal" JavaScript multiplication if both parameters are < 32767
*
* Refer to: http://stackoverflow.com/questions/13597364/32-bit-signed-multiplication-with-a-64-bit-result-in-javascript
*
* @this {X86CPU}
* @param {number} dst (any 32-bit number, treated as unsigned)
* @param {number} src (any 32-bit number, treated as unsigned)
*/
X86.fnMUL32 = function(dst, src)
{
if (!(dst & ~0xffff) && !(src & ~0xffff)) {
this.fMDSet = true;
this.regMDLo = (dst * src)|0;
this.regMDHi = 0;
return;
}
var srcLo = src & 0xffff;
var srcHi = src >>> 16;
var dstLo = dst & 0xffff;

View file

@ -76,6 +76,7 @@ if (NODE) {
global.window = false; // provides an alternative "if (typeof window === 'undefined')" (ie, "if (window) ...")
global.APPNAME = APPNAME;
global.APPVERSION = APPVERSION;
global.XMLVERSION = XMLVERSION;
global.SITEHOST = SITEHOST;
global.COMPILED = COMPILED;
global.DEBUG = DEBUG;

View file

@ -1,7 +1,10 @@
all: test386.json
all: test386.json test386.img
test386.com: test386.nasm ../inc/dos.inc ../inc/misc.inc ../inc/x86.inc
nasm -i../inc/ -f bin test386.nasm -l test386.lst -o test386.com
test386.json: test386.com
node ../../../modules/filedump/bin/filedump --file=test386.com --output=test386.json --overwrite
test386.img: test386.com
node ../../../modules/diskdump/bin/diskdump --path=test386.com --output=test386.img --format=img --overwrite

View file

@ -146,8 +146,11 @@ start: nop
;
mov sp,tempStack
;
; Quick test of unsigned 32-bit multiplication and division
; Quick tests of unsigned 32-bit multiplication and division
;
mov eax,0x80000001
imul eax
mov eax,0x44332211
mov ebx,eax
mov ecx,0x88776655
@ -719,23 +722,35 @@ printVal:
ret
TYPE_ARITH equ 0
TYPE_LOGIC equ 1
TYPE_ARITH1 equ 1
TYPE_LOGIC equ 2
TYPE_MULDIV equ 3
SIZE_BYTE equ 0
SIZE_SHORT equ 1
SIZE_LONG equ 2
%macro defOp 5
%macro defOp 6
%ifidni %3,al
%assign size SIZE_BYTE
%elifidni %3,dl
%assign size SIZE_BYTE
%elifidni %3,ax
%assign size SIZE_SHORT
%elifidni %3,dx
%assign size SIZE_SHORT
%else
%assign size SIZE_LONG
%endif
db %%end-%%beg,%5,size
%%name: db %1,0
%%beg: %2 %3,%4
db %%end-%%beg,%6,size
%%name:
db %1,0
%%beg:
%ifidni %4,none
%2 %3
%else
%2 %3,%4
%endif
ret
%%end:
%endmacro
@ -746,37 +761,50 @@ strPS: db "PS=",0
achSize db "BWD"
tableOps:
defOp "ADD",add,al,dl,TYPE_ARITH
defOp "ADD",add,ax,dx,TYPE_ARITH
defOp "ADD",add,eax,edx,TYPE_ARITH
defOp "OR",or,al,dl,TYPE_LOGIC
defOp "OR",or,ax,dx,TYPE_LOGIC
defOp "OR",or,eax,edx,TYPE_LOGIC
defOp "ADC",adc,al,dl,TYPE_ARITH
defOp "ADC",adc,ax,dx,TYPE_ARITH
defOp "ADC",adc,eax,edx,TYPE_ARITH
defOp "SBB",sbb,al,dl,TYPE_ARITH
defOp "SBB",sbb,ax,dx,TYPE_ARITH
defOp "SBB",sbb,eax,edx,TYPE_ARITH
defOp "AND",and,al,dl,TYPE_LOGIC
defOp "AND",and,ax,dx,TYPE_LOGIC
defOp "AND",and,eax,edx,TYPE_LOGIC
defOp "SUB",sub,al,dl,TYPE_ARITH
defOp "SUB",sub,ax,dx,TYPE_ARITH
defOp "SUB",sub,eax,edx,TYPE_ARITH
defOp "XOR",xor,al,dl,TYPE_LOGIC
defOp "XOR",xor,ax,dx,TYPE_LOGIC
defOp "XOR",xor,eax,edx,TYPE_LOGIC
defOp "CMP",cmp,al,dl,TYPE_ARITH
defOp "CMP",cmp,ax,dx,TYPE_ARITH
defOp "CMP",cmp,eax,edx,TYPE_ARITH
defOp "ADD",add,al,dl,none,TYPE_ARITH
defOp "ADD",add,ax,dx,none,TYPE_ARITH
defOp "ADD",add,eax,edx,none,TYPE_ARITH
defOp "OR",or,al,dl,none,TYPE_LOGIC
defOp "OR",or,ax,dx,none,TYPE_LOGIC
defOp "OR",or,eax,edx,none,TYPE_LOGIC
defOp "ADC",adc,al,dl,none,TYPE_ARITH
defOp "ADC",adc,ax,dx,none,TYPE_ARITH
defOp "ADC",adc,eax,edx,none,TYPE_ARITH
defOp "SBB",sbb,al,dl,none,TYPE_ARITH
defOp "SBB",sbb,ax,dx,none,TYPE_ARITH
defOp "SBB",sbb,eax,edx,none,TYPE_ARITH
defOp "AND",and,al,dl,none,TYPE_LOGIC
defOp "AND",and,ax,dx,none,TYPE_LOGIC
defOp "AND",and,eax,edx,none,TYPE_LOGIC
defOp "SUB",sub,al,dl,none,TYPE_ARITH
defOp "SUB",sub,ax,dx,none,TYPE_ARITH
defOp "SUB",sub,eax,edx,none,TYPE_ARITH
defOp "XOR",xor,al,dl,none,TYPE_LOGIC
defOp "XOR",xor,ax,dx,none,TYPE_LOGIC
defOp "XOR",xor,eax,edx,none,TYPE_LOGIC
defOp "CMP",cmp,al,dl,none,TYPE_ARITH
defOp "CMP",cmp,ax,dx,none,TYPE_ARITH
defOp "CMP",cmp,eax,edx,none,TYPE_ARITH
defOp "INC",inc,al,none,none,TYPE_ARITH1
defOp "INC",inc,ax,none,none,TYPE_ARITH1
defOp "INC",inc,eax,none,none,TYPE_ARITH1
defOp "DEC",dec,al,none,none,TYPE_ARITH1
defOp "DEC",dec,ax,none,none,TYPE_ARITH1
defOp "DEC",dec,eax,none,none,TYPE_ARITH1
defOp "IMULA",imul,dl,none,none,TYPE_MULDIV
defOp "IMULA",imul,dx,none,none,TYPE_MULDIV
defOp "IMULA",imul,edx,none,none,TYPE_MULDIV
defOp "IMUL",imul,ax,dx,none,TYPE_MULDIV
defOp "IMUL",imul,eax,edx,none,TYPE_MULDIV
db 0
align 4
typeMasks:
dd PS_ARITH
dd PS_ARITH
dd PS_LOGIC
dd PS_MULDIV
arithValues:
.bvals: dd 0x00,0x01,0x02,0x7E,0x7F,0x80,0x81,0xFE,0xFF
@ -788,6 +816,16 @@ arithValues:
.dvals: dd 0x00000000,0x00000001,0x00000002,0x7FFFFFFE,0x7FFFFFFF,0x80000000,0x80000001,0xFFFFFFFE,0xFFFFFFFF
ARITH_DWORDS equ ($-.dvals)/4
muldivValues:
.bvals: dd 0x00,0x01,0x02,0x3F,0x40,0x41,0x7E,0x7F,0x80,0x81,0xFE,0xFF
MULDIV_BYTES equ ($-.bvals)/4
.wvals: dd 0x0000,0x0001,0x0002,0x3FFF,0x4000,0x4001,0x7FFE,0x7FFF,0x8000,0x8001,0xFFFE,0xFFFF
MULDIV_WORDS equ ($-.wvals)/4
.dvals: dd 0x00000000,0x00000001,0x00000002,0x3FFFFFFF,0x40000000,0x40000001,0x7FFFFFFE,0x7FFFFFFF,0x80000000,0x80000001,0xFFFFFFFE,0xFFFFFFFF
MULDIV_DWORDS equ ($-.dvals)/4
typeValues:
;
; Values for TYPE_ARITH
@ -797,12 +835,26 @@ typeValues:
dd ARITH_BYTES+ARITH_WORDS+ARITH_DWORDS,arithValues,ARITH_BYTES+ARITH_WORDS+ARITH_DWORDS,arithValues
dd 0,0,0,0
;
; Values for TYPE_LOGIC (I'm using ARITH values for now)
; Values for TYPE_ARITH1
;
dd ARITH_BYTES,arithValues,1,arithValues
dd ARITH_BYTES+ARITH_WORDS,arithValues,1,arithValues
dd ARITH_BYTES+ARITH_WORDS+ARITH_DWORDS,arithValues,1,arithValues
dd 0,0,0,0
;
; Values for TYPE_LOGIC (using ARITH values for now)
;
dd ARITH_BYTES,arithValues,ARITH_BYTES,arithValues
dd ARITH_BYTES+ARITH_WORDS,arithValues,ARITH_BYTES+ARITH_WORDS,arithValues
dd ARITH_BYTES+ARITH_WORDS+ARITH_DWORDS,arithValues,ARITH_BYTES+ARITH_WORDS+ARITH_DWORDS,arithValues
dd 0,0,0,0
;
; Values for TYPE_MULDIV (a superset of ARITH values)
;
dd MULDIV_BYTES,muldivValues,MULDIV_BYTES,muldivValues
dd MULDIV_BYTES+MULDIV_WORDS,muldivValues,MULDIV_BYTES+MULDIV_WORDS,muldivValues
dd MULDIV_BYTES+MULDIV_WORDS+MULDIV_DWORDS,muldivValues,MULDIV_BYTES+MULDIV_WORDS+MULDIV_DWORDS,muldivValues
dd 0,0,0,0
error: jmp error

View file

@ -12,6 +12,7 @@ PS_DF equ 0x0400
PS_OF equ 0x0800
PS_ARITH equ (PS_CF | PS_PF | PS_AF | PS_ZF | PS_SF | PS_OF)
PS_LOGIC equ (PS_CF | PS_PF | PS_ZF | PS_SF | PS_OF)
PS_MULDIV equ (PS_CF | PS_OF)
CR0_MSW_PE equ 0x0001
CR0_PG equ 0x80000000 ; set if paging enabled