https://codereview.chromium.org/633423002/diff/20001/src/compiler/ast-graph-builder.h
File src/compiler/ast-graph-builder.h (right):

https://codereview.chromium.org/633423002/diff/20001/src/compiler/ast-graph-builder.h#newcode55
src/compiler/ast-graph-builder.h:55: LoadICFeedbackNode
LoadNamedFeedback(int slot) const {
As discussed offline: Haveing to deal with IC-related data structures
here is a smell. Can we instead just define a vecotr-slot-pair in
js-operators.h that is a pure data container without logic?

nit: This needs a comment. Also we should make it private (by placing it
further down).

https://codereview.chromium.org/633423002/diff/20001/src/compiler/ast-graph-builder.h#newcode93
src/compiler/ast-graph-builder.h:93: Node*
BuildVariableLoad(VariableProxy* proxy, BailoutId bailout_id,
Either all the BuildVariableFoo helpers should work on Variable or on
VariableProxy, this arbitrary dichotomy smells.

As discussed offline: It might be better to instead pass the
vector-slot-pair as an argument.

https://codereview.chromium.org/633423002/diff/20001/src/compiler/ast-graph-builder.h#newcode129
src/compiler/ast-graph-builder.h:129: Handle<TypeFeedbackVector>
vector_;
nit: Let's not start caching handles yet, this seems like premature
optimization.

https://codereview.chromium.org/633423002/diff/20001/test/unittests/compiler/js-typed-lowering-unittest.cc
File test/unittests/compiler/js-typed-lowering-unittest.cc (right):

https://codereview.chromium.org/633423002/diff/20001/test/unittests/compiler/js-typed-lowering-unittest.cc#newcode90
test/unittests/compiler/js-typed-lowering-unittest.cc:90:
KeyedLoadICFeedbackNode feedback(Handle<TypeFeedbackVector>::null(), 0);
nit: Please move this out of the for-each loop (to around line 80) for
readability.

https://codereview.chromium.org/633423002/

--
--
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