The problem is that you're missing recording of the source position in the
full-codegen. But it is easy to fix. I sent a few small comments on the code
review as well.
Until now we had no need for that, since the old UnaryOpStub was not an IC
- the
type oracle iterates over all calls to IC and has the implicit assumption
that
there is a source position recorded for each IC call. If not, things go
wrong -
i.e. it will record two ICs for the same source position etc. resulting in
missing typefeedback for some operations.
You need to add a position_ member to the UnaryOperation class in ast.h. In
parser.cc you need to pass the position into the constructor of
UnaryOperation
(like we do e.g. for BinaryOperation)
http://codereview.chromium.org/6879081/diff/10001/src/ia32/code-stubs-ia32.cc
File src/ia32/code-stubs-ia32.cc (right):
http://codereview.chromium.org/6879081/diff/10001/src/ia32/code-stubs-ia32.cc#newcode719
src/ia32/code-stubs-ia32.cc:719:
Remove one extra line.
http://codereview.chromium.org/6879081/diff/10001/src/ia32/code-stubs-ia32.h
File src/ia32/code-stubs-ia32.h (right):
http://codereview.chromium.org/6879081/diff/10001/src/ia32/code-stubs-ia32.h#newcode143
src/ia32/code-stubs-ia32.h:143: void
GenerateHeapNumberCodeSub(MacroAssembler* masm, // Label* undo,
Remove commented code + fit on one line.
http://codereview.chromium.org/6879081/diff/10001/src/ia32/code-stubs-ia32.h#newcode145
src/ia32/code-stubs-ia32.h:145: void
GenerateHeapNumberCodeBitNot(MacroAssembler* masm, // Label* undo,
Remove commented code + fit on one line.
http://codereview.chromium.org/6879081/diff/10001/src/ia32/full-codegen-ia32.cc
File src/ia32/full-codegen-ia32.cc (right):
http://codereview.chromium.org/6879081/diff/10001/src/ia32/full-codegen-ia32.cc#newcode3744
src/ia32/full-codegen-ia32.cc:3744: // TODO(svenpanne): Allowing format
strings in Comment would be nice here...
It's a little complicated to do, since format strings require allocating
a buffer in JS heap since comments are referenced by the reloc-info... -
so I'm not sure if you should make it a TODO now.
http://codereview.chromium.org/6879081/diff/10001/src/ia32/full-codegen-ia32.cc#newcode3752
src/ia32/full-codegen-ia32.cc:3752:
VisitForAccumulatorValue(expr->expression());
You need to call here:
SetSourcePosition(expr->position())
This will record the statement position which we use to associate the
type
feedback with. We have the same call to SetSourcePosition for binary
operations.
http://codereview.chromium.org/6879081/diff/10001/src/ia32/full-codegen-ia32.cc#newcode3753
src/ia32/full-codegen-ia32.cc:3753: __ CallStub(&stub);
You should use here:
EmitCallIC(stub.GetCode(), NULL)
where NULL signals that there is no inlined smi code.
Since we probably want to add patchable inline smi code later (like we
already do for binary ops)
http://codereview.chromium.org/6879081/diff/10001/src/ic.cc
File src/ic.cc (right):
http://codereview.chromium.org/6879081/diff/10001/src/ic.cc#newcode1819
src/ic.cc:1819: case SMI: return "SMI";
--> "Smi"
http://codereview.chromium.org/6879081/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
