Fixed type feedback in presence of negative lookups.

Fixed type feedback in presence of negative lookups.

To fix the issue at hand regarding constant function calls and perhaps
other hidden issues regarding negative lookups, we basically add a
"marker instruction", just for harvesting purposes. Our type feedback
oracle is really, really fragile, we should better switch to some more
explicit and robust scheme soon.

BUG=chromium:252797
R=mstarzinger@chromium.org

Committed: http://code.google.com/p/v8/source/detail?r=15741

[Michael Starzinger, 2013-07-17 15:03:30]

https://codereview.chromium.org/19588002/diff/2001/src/ia32/stub-cache-ia32.cc
File src/ia32/stub-cache-ia32.cc (right):

https://codereview.chromium.org/19588002/diff/2001/src/ia32/stub-cache-ia32.c...
src/ia32/stub-cache-ia32.cc:1136: __ mov(scratch1, Handle<Map>(object->map()));
The StubCompiler::CheckPrototypes() helper is used in many places, even several
times within the same stub. So I think this should be hoisted out of this helper
to only add the embedded map at the top of each stub that actually needs it.

[Michael Starzinger, 2013-07-17 15:15:17]

https://codereview.chromium.org/19588002/diff/2001/src/ia32/stub-cache-ia32.cc
File src/ia32/stub-cache-ia32.cc (right):

https://codereview.chromium.org/19588002/diff/2001/src/ia32/stub-cache-ia32.c...
src/ia32/stub-cache-ia32.cc:1136: __ mov(scratch1, Handle<Map>(object->map()));
On 2013/07/17 15:03:30, Michael Starzinger wrote:
> The StubCompiler::CheckPrototypes() helper is used in many places, even
several
> times within the same stub. So I think this should be hoisted out of this
helper
> to only add the embedded map at the top of each stub that actually needs it.

To be precise, this should go into CallStubCompiler::CompileCallConstant(). Toon
also made a good point, the CheckPrototypes is also used for loads and stores,
not just calls.

[Sven Panne, 2013-07-18 09:01:29]

As discussed offline, there is basically no way to statically determine all code
paths which would need a modification. Therefore, we'll keep the "marker
instruction" in the central place, even if it's redundant for some call paths.
There is now a TODO to remove this when all ICs are reworked.

In a nutshell: The CL at hand is conservative and shows no performance
degradation, so keep it as it is for a transitional phase.

[Michael Starzinger, 2013-07-18 09:07:25]

LGTM. You convinced me.

As discussed offline: If the performance impact of this change is negligible and
with the added TODO, I am fine to land the change as it is. I would still
double-check with Toon though and let him know about the TODO.

[Sven Panne, 2013-07-18 09:12:51]

Committed patchset #3 manually as r15741 (presubmit successful).