Hi Michael, thanks for the look. Here is a new patch!
--Michael
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 {
On 2014/10/08 15:23:09, Michael Starzinger wrote:
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).
Done.
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,
On 2014/10/08 15:23:09, Michael Starzinger wrote:
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.
Done.
https://codereview.chromium.org/633423002/diff/20001/src/compiler/ast-graph-builder.h#newcode129
src/compiler/ast-graph-builder.h:129: Handle<TypeFeedbackVector>
vector_;
On 2014/10/08 15:23:09, Michael Starzinger wrote:
nit: Let's not start caching handles yet, this seems like premature
optimization.
Done.
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);
On 2014/10/08 15:23:09, Michael Starzinger wrote:
nit: Please move this out of the for-each loop (to around line 80) for
readability.
Done.
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.
