https://codereview.chromium.org/12049012/diff/2001/src/handles-inl.h File src/handles-inl.h (right):
https://codereview.chromium.org/12049012/diff/2001/src/handles-inl.h#newcode183 src/handles-inl.h:183: active_ = !isolate->optimizing_compiler_thread()->IsOptimizerThread(); I'd appreciate a comment here, something along the lines of: "The guard can only be enabled for all threads at once, so we can only do that when parallel compilation is disabled. Checking for the current thread is sufficient to detect that, because this constructor is only ever called from the optimizer thread." While typing this it occurred to me: why not just hook this off the FLAG_parallel_recompilation? https://codereview.chromium.org/12049012/diff/2001/src/handles.h File src/handles.h (right): https://codereview.chromium.org/12049012/diff/2001/src/handles.h#newcode68 src/handles.h:68: return *location_ == *other.location(); Why "location_" vs "location()"? Let's be consistent. https://codereview.chromium.org/12049012/diff/2001/src/handles.h#newcode352 src/handles.h:352: int old_state_; This is used to store bools, so it should be a bool. https://codereview.chromium.org/12049012/diff/2001/src/handles.h#newcode367 src/handles.h:367: int old_state_; Same here. https://codereview.chromium.org/12049012/diff/2001/src/hydrogen-instructions.h File src/hydrogen-instructions.h (right): https://codereview.chromium.org/12049012/diff/2001/src/hydrogen-instructions.h#newcode2671 src/hydrogen-instructions.h:2671: // HConstant(const HConstant&); what's this? https://codereview.chromium.org/12049012/diff/2001/src/hydrogen-instructions.h#newcode2701 src/hydrogen-instructions.h:2701: if (*handle_.location() == Object::cast(heap->undefined_value()) || "if (a || b) {return true;} return false;" is kind of equivalent to "return (a || b);", no? I don't feel strongly about it; if you think this is more readable feel free to leave it as it is. https://codereview.chromium.org/12049012/diff/2001/src/hydrogen-instructions.h#newcode2761 src/hydrogen-instructions.h:2761: hash = reinterpret_cast<intptr_t>(*handle_.location()); So, dereferencing the handle is not allowed, but dereferencing its location() is? Who are we kidding? https://codereview.chromium.org/12049012/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
