LGTM.

https://codereview.chromium.org/1220823004/diff/1/src/compiler/context-relaxation.cc
File src/compiler/context-relaxation.cc (right):

https://codereview.chromium.org/1220823004/diff/1/src/compiler/context-relaxation.cc#newcode14
src/compiler/context-relaxation.cc:14: Reduction
ContextRelaxation::Reduce(Node* node) {
I think it would make sense to rename this class to JSContextRelaxation
and the file to js-context-relaxation.cc, WDYT?

https://codereview.chromium.org/1220823004/diff/1/src/compiler/context-relaxation.cc#newcode28
src/compiler/context-relaxation.cc:28: if (!context_is_wired_) {
On 2015/07/01 09:11:48, danno wrote:
This is a bit weird, any better ideas?

As discussed offline: We could move the parameter into the JSGraph,
thereby graph trimming would not kill it. Beut we can do that in a
follow-up CL.

https://codereview.chromium.org/1220823004/diff/1/src/compiler/context-relaxation.cc#newcode29
src/compiler/context-relaxation.cc:29: if (relaxed_context_->InputAt(0)
== nullptr) {
Can we please use relaxed_context_->IsDead() instead?

https://codereview.chromium.org/1220823004/diff/1/src/compiler/context-relaxation.h
File src/compiler/context-relaxation.h (right):

https://codereview.chromium.org/1220823004/diff/1/src/compiler/context-relaxation.h#newcode17
src/compiler/context-relaxation.h:17: // builder. The makes it possible
to use they operations with context
nit: s/The/This/ and s/they/these/

https://codereview.chromium.org/1220823004/diff/1/src/compiler/context-relaxation.h#newcode18
src/compiler/context-relaxation.h:18: // specialization (e.g. for
generating stubs) without embedding forcing inner
nit: s/embedding//

https://codereview.chromium.org/1220823004/diff/1/src/compiler/context-relaxation.h#newcode22
src/compiler/context-relaxation.h:22: explicit ContextRelaxation(Node*
start, Node* relaxed_context)
nit: Doesn't need to be marked "explicit".

https://codereview.chromium.org/1220823004/diff/1/src/compiler/pipeline.cc
File src/compiler/pipeline.cc (right):

https://codereview.chromium.org/1220823004/diff/1/src/compiler/pipeline.cc#newcode295
src/compiler/pipeline.cc:295: Node* relaxed_context_;
nit: Please reset this to {nullptr} in PipelineData::DeleteGraphZone.

https://codereview.chromium.org/1220823004/

--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to