https://codereview.chromium.org/1021713005/diff/20001/src/compiler/ast-graph-builder.cc
File src/compiler/ast-graph-builder.cc (right):
https://codereview.chromium.org/1021713005/diff/20001/src/compiler/ast-graph-builder.cc#newcode1938
src/compiler/ast-graph-builder.cc:1938: BuildNamedLoad(object, name,
pair, expr->AssignmentFeedbackId());
On 2015/03/24 12:11:18, Michael Starzinger wrote:
On 2015/03/24 10:49:31, titzer wrote:
> On 2015/03/24 09:06:40, Michael Starzinger wrote:
> > Shouldn't the property loads in the assignment use
> > property->PropertyFeedbackId() here?
>
> I cribbed from typing.cc which adds the type feedback to the AST for
Crankshaft,
> and it uses the AssignmentFeedbackId.
As discussed offline: In typing.cc the AstTyper::VisitAssignment
recurses on
expr->target, which in turn will go into AstTyper::VisitProperty where
expr->target()->PropertyFeedbackId() is used.
Done.
https://codereview.chromium.org/1021713005/diff/20001/src/compiler/ast-graph-builder.cc#newcode1949
src/compiler/ast-graph-builder.cc:1949: BuildKeyedLoad(object, key,
pair, expr->AssignmentFeedbackId());
On 2015/03/24 12:11:17, Michael Starzinger wrote:
On 2015/03/24 10:49:31, titzer wrote:
> On 2015/03/24 09:06:40, Michael Starzinger wrote:
> > Likewise.
>
> Same as above.
Likewise.
Done.
https://codereview.chromium.org/1021713005/diff/20001/src/compiler/js-type-feedback.cc
File src/compiler/js-type-feedback.cc (right):
https://codereview.chromium.org/1021713005/diff/20001/src/compiler/js-type-feedback.cc#newcode87
src/compiler/js-type-feedback.cc:87: if (property_details.cell_type() ==
PropertyCellType::kConstant) {
On 2015/03/24 12:56:12, Toon Verwaest wrote:
On 2015/03/24 10:49:31, titzer wrote:
> On 2015/03/24 09:42:39, Toon Verwaest wrote:
> > This is not how constants are encoded. PropertyDetails have 3
partially
> distinct
> > encodings. Dictionary, global, and fast-mode objects. Dictionary
and global
> are
> > mostly the same, minus that global also has PropertyCellType. Here
we only
> care
> > about fast-mode objects though. For now you probably want to
disallow
> everything
> > that's property_details.type() != DATA, which includes
DATA_CONSTANT, and
both
> > kinds of accessors.
>
> So the condition should be "property_details.type() != DATA"?
indeed
Done.
https://codereview.chromium.org/1021713005/diff/20001/src/compiler/js-type-feedback.cc#newcode94
src/compiler/js-type-feedback.cc:94: // TODO(turbofan): constants using
code dependencies.
On 2015/03/24 12:56:12, Toon Verwaest wrote:
On 2015/03/24 10:49:32, titzer wrote:
> On 2015/03/24 09:42:39, Toon Verwaest wrote:
> > Fast-mode-map-based constants don't use code dependencies. The map
will
> change.
>
> So I can just remove this condition?
>
> The actual threading through of what constant to load needs to be
thought out
> into an abstractification.
Indeed. This condition makes no sense. You'll again need to do if
(property_details.type() != DATA) return false;
Done.
https://codereview.chromium.org/1021713005/diff/20001/src/flag-definitions.h
File src/flag-definitions.h (right):
https://codereview.chromium.org/1021713005/diff/20001/src/flag-definitions.h#newcode407
src/flag-definitions.h:407: DEFINE_IMPLICATION(turbo_type_feedback,
turbo_deoptimization)
On 2015/03/24 12:11:18, Michael Starzinger wrote:
On 2015/03/24 10:49:32, titzer wrote:
> Comments on this? Should type feedback imply deoptimization support?
We could
> still support fast path map checks and guarded inlining without
deoptimizing,
> but nothing that requires code dependencies.
Agreed, it would be nice to be able to leverage dynamic feedback even
without
deoptimization support. Not sure if we ever want to run in such a
mode, but it
would be nice to know that we could. :)
I removed the implication and put a branch in js-type-feedback.cc
https://codereview.chromium.org/1021713005/
--
--
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.
