http://codereview.chromium.org/652041/diff/19/1027 File src/ia32/codegen-ia32.cc (right):
http://codereview.chromium.org/652041/diff/19/1027#newcode8174 src/ia32/codegen-ia32.cc:8174: __ fld_d(FieldOperand(eax, HeapNumber::kValueOffset)); Probably. I would need a flag, or two different paths, to know whether I should load the double value later. I would also need to remember the pointer to the heap number (which I can otherwise drop after these loads). In any case, fld is a very cheap operation (for the CPU, it just onloads it on the FPU and waits for it to complete there). I don't think it's worth complicating the code for. http://codereview.chromium.org/652041/diff/19/1027#newcode8192 src/ia32/codegen-ia32.cc:8192: __ and_(Operand(ecx), Immediate(TranscendentalCache::kCacheSize - 1)); Well spotted. I moved this line up here but forgot the assert. http://codereview.chromium.org/652041/diff/19/1027#newcode8228 src/ia32/codegen-ia32.cc:8228: __ cmp(edx, Operand(ecx, kIntSize)); // NOLINT It should be. The cache element holds two integers, which together should be all the bits of the double. The kIntSize here matches the layout test in the DEBUG section above. The NOLINT should go, in any case. It no longer reads sizeof(int32_t). http://codereview.chromium.org/652041/diff/19/1027#newcode8232 src/ia32/codegen-ia32.cc:8232: __ fstp(0); I don't think it's worth it. In the smi case, I need to convert the smi to a double, so the fldi does double service by both converting and pushing the value. The fstp operation is very quick as well (often just 1 cycle on modern chips). http://codereview.chromium.org/652041/diff/19/1027#newcode8271 src/ia32/codegen-ia32.cc:8271: case TranscendentalCache::COS: { There are potentially more, some of which won't need the same setup as the 2PI periodic ones, but there's no need for the generality yet. I'll make it into an ASSERT and make a comment about future extensions. http://codereview.chromium.org/652041/diff/19/1027#newcode8320 src/ia32/codegen-ia32.cc:8320: __ fprem(); It is slightly slower on some chips, but it also gives a slightly more precise result. I guess the tradeof should go in the direction of precission in this case. I'll change it to fprem1. http://codereview.chromium.org/652041/diff/19/1029 File src/ia32/disasm-ia32.cc (right): http://codereview.chromium.org/652041/diff/19/1029#newcode681 src/ia32/disasm-ia32.cc:681: case 0: mnem = "fld_`d"; break; fixed. http://codereview.chromium.org/652041 -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
