https://codereview.chromium.org/1021713005/diff/20001/src/compiler.cc
File src/compiler.cc (right):
https://codereview.chromium.org/1021713005/diff/20001/src/compiler.cc#newcode421
src/compiler.cc:421: } else if (FLAG_turbo_type_feedback) {
On 2015/03/24 10:49:31, titzer wrote:
On 2015/03/24 09:06:39, Michael Starzinger wrote:
> Can we please move this into the CompilationInfo constructor?
As discussed in person, we only want to enable type feedback for
non-asm
functions at the moment.
Acknowledged.
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 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.
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 10:49:31, titzer wrote:
On 2015/03/24 09:06:40, Michael Starzinger wrote:
> Likewise.
Same as above.
Likewise.
https://codereview.chromium.org/1021713005/diff/20001/src/compiler/ast-graph-builder.h
File src/compiler/ast-graph-builder.h (right):
https://codereview.chromium.org/1021713005/diff/20001/src/compiler/ast-graph-builder.h#newcode271
src/compiler/ast-graph-builder.h:271: const VectorSlotPair& feedback,
TypeFeedbackId id);
On 2015/03/24 10:49:31, titzer wrote:
On 2015/03/24 09:06:40, Michael Starzinger wrote:
> suggestion: Would it make sense to make TypeFeedbackId::None() a
default
> argument for all four of these methods?
I prefer the explicit usage of TypeFeedbackId::None(), since that
marks
locations where we will want to add new id's in the future.
Acknowledged.
https://codereview.chromium.org/1021713005/diff/20001/src/compiler/verifier.cc
File src/compiler/verifier.cc (right):
https://codereview.chromium.org/1021713005/diff/20001/src/compiler/verifier.cc#newcode622
src/compiler/verifier.cc:622: /* TODO(titzer)
On 2015/03/24 10:49:32, titzer wrote:
Haha, no one noticed this!
Maeh, verifier, shmerifier. :)
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 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. :)
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.
