I'll upload a tiny CL for this stuff now...
http://codereview.chromium.org/7461107/diff/9001/src/ia32/lithium-codegen-ia32.cc File src/ia32/lithium-codegen-ia32.cc (right): http://codereview.chromium.org/7461107/diff/9001/src/ia32/lithium-codegen-ia32.cc#newcode1395 src/ia32/lithium-codegen-ia32.cc:1395: Factory* factory = this->factory(); On 2011/07/28 13:54:58, fschneider wrote:
Doesn't the compiler inline the factory() accessor?
Ooops, I haven't seen that retrieving the current isolate is cheap here, because we can get it via an instance field. So this is local variable is probably not needed, I'll remove it. http://codereview.chromium.org/7461107/diff/9001/src/ia32/lithium-codegen-ia32.cc#newcode1406 src/ia32/lithium-codegen-ia32.cc:1406: if (expected.IsEmpty()) expected = ToBooleanStub::all_types(); On 2011/07/28 13:54:58, fschneider wrote:
We should watch out for code size a little. If this case happens
often, we will
generate lots of inline code. Can you compare performance when calling
the stub
in this case?
In the programs I've seen so far this doesn't happen very often, but when it happens, it happens in "hot" code a lot, so I would really like to avoid the overhead of calling a stub. What would really help regarding code size and speed would be a different handling of undetectable objects, but that is a different story... http://codereview.chromium.org/7461107/diff/9001/src/ia32/lithium-codegen-ia32.cc#newcode1423 src/ia32/lithium-codegen-ia32.cc:1423: // We've seen a string for the first time -> deopt. On 2011/07/28 13:54:58, fschneider wrote:
Update comment: s/string/boolean
Done. http://codereview.chromium.org/7461107/diff/9001/src/ia32/lithium-codegen-ia32.cc#newcode1433 src/ia32/lithium-codegen-ia32.cc:1433: // We've seen a string for the first time -> deopt. On 2011/07/28 13:54:58, fschneider wrote:
Same here.
Done. http://codereview.chromium.org/7461107/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
