LGTM as well.
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(); Doesn't the compiler inline the factory() accessor? 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(); 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? 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. Update comment: s/string/boolean 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. Same here. http://codereview.chromium.org/7461107/diff/9001/src/ia32/lithium-ia32.cc File src/ia32/lithium-ia32.cc (right): http://codereview.chromium.org/7461107/diff/9001/src/ia32/lithium-ia32.cc#newcode1048 src/ia32/lithium-ia32.cc:1048: return AssignEnvironment(branch); If we cover all cases, then we don't deopt, right? In this case you can avoid assigning an environment to reduce the number of live values. http://codereview.chromium.org/7461107/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
