ibmruntimes / v8ppc

Port of Google V8 javascript engine to PowerPC®
Other
94 stars 19 forks source link

Simple mod operation failing #76

Closed mtbrandy closed 11 years ago

mtbrandy commented 11 years ago

This problem was originally referenced in issue #68.

$ cat test3.js

function test(x) { 
  y = x % 1; 
  print(x + " % 1 = " + y); 
} 
test(0xffffffff); 
test(0xffffffff); 
test(0xffffffff); 

$ out/ppc.debug/d8 test3.js

4294967295 % 1 = 0 
4294967295 % 1 = 2.300997624e-315 
4294967295 % 1 = 2.300997624e-315 
tunniclm commented 11 years ago

I tried adding tracing around the call to fmod() and found something curious: modulo(4294967295, 1) = 0 4294967295 % 1 = 0 modulo(4294967295, 3.6453292507556801e-314) = 2.3169104905565888e-315 4294967295 % 1 = 2.31691049e-315 modulo(4294967295, 3.6453292507556801e-314) = 2.3169104905565888e-315 4294967295 % 1 = 2.31691049e-315

(NB: I'm using "%.17g" format to printf() for the trace output to get good precision.)

tunniclm commented 11 years ago

In this case the first (correct) call to modulo() comes out of the runtime, the subsequent (broken) ones out of generated code.

tunniclm commented 11 years ago

OK, the problem appears to be a mess of conversion code (LoadNumber()). There is a conversion from Object field/SMI to FP and then back again which doesn't look necessary. The problem is in the conversion TO floating point - in the case where the value is an SMI and not an Object field, the LoadNumber() code ignores the desired destination register and always stores to FP register. In the failing case we want the SMI to be converted to FP and stored in 2 GPRs.

tunniclm commented 11 years ago

To clarify, the unnecessary part I talked about above is not the conversion from Object field/SMI to FP, it is the storing of the FP representation first into a pair of GPRs in LoadOperands() (which calls LoadNumber()) then a short while later loading that into a FPR (in CallCCodeForDoubleOperation()). We should probably be loading it straight into a FPR (however, currently CallCCodeForDoubleOperation() expects it in two GPRs.)

dstence commented 11 years ago

Fixed via commit 9309c6e23cdcbe19b34aaa0fcca7750e74f4b2e2.