New issue
Advanced search Search tips
Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 21
Cc:
Components:
HW: All
NextAction: ----
OS: All
Priority: 1
Type: Bug

Blocking:
issue 6936



Sign in to add a comment

Investigate potential deoptimization loop with KeyedLoadIC feedback in the jshint test

Project Member Reported by bmeu...@chromium.org, Oct 17 Back to list

Issue description

In the JSHint test of the web-tooling-benchmark we seem to hit some kind of deoptimization loop with the involving the block and the directives function. See the attached graph:

<wtb-jshint.js:48381:26> inlined at <wtb-jshint.js:48446:11>	Reason: index and name do not match in access
<wtb-jshint.js:48381:26> inlined at <wtb-jshint.js:48446:11>	Reason: index and name do not match in access
<wtb-jshint.js:48381:26> inlined at <wtb-jshint.js:48446:11>	Reason: index and name do not match in access
<wtb-jshint.js:48381:26> inlined at <wtb-jshint.js:48446:11>	Reason: index and name do not match in access
<wtb-jshint.js:48381:26> inlined at <wtb-jshint.js:48446:11>	Reason: index and name do not match in access
<wtb-jshint.js:48381:26> inlined at <wtb-jshint.js:48446:11>	Reason: index and name do not match in access
<wtb-jshint.js:48381:26> inlined at <wtb-jshint.js:48446:11>	Reason: index and name do not match in access
<wtb-jshint.js:48381:26> inlined at <wtb-jshint.js:48446:11>	Reason: index and name do not match in access
<wtb-jshint.js:48381:26> inlined at <wtb-jshint.js:48446:11>	Reason: index and name do not match in access
<wtb-jshint.js:48381:26> inlined at <wtb-jshint.js:48446:11>	Reason: index and name do not match in access
<wtb-jshint.js:48381:26> inlined at <wtb-jshint.js:48446:11>	Reason: index and name do not match in access
<wtb-jshint.js:48381:26> inlined at <wtb-jshint.js:48446:11>	Reason: index and name do not match in access
<wtb-jshint.js:48381:26> inlined at <wtb-jshint.js:48446:11>	Reason: index and name do not match in access
<wtb-jshint.js:48381:26> inlined at <wtb-jshint.js:48446:11>	Reason: index and name do not match in access
<wtb-jshint.js:48381:26> inlined at <wtb-jshint.js:48446:11>	Reason: index and name do not match in access
<wtb-jshint.js:48381:26> inlined at <wtb-jshint.js:48446:11>	Reason: index and name do not match in access
<wtb-jshint.js:48381:26> inlined at <wtb-jshint.js:48446:11>	Reason: index and name do not match in access
<wtb-jshint.js:48381:26> inlined at <wtb-jshint.js:48446:11>	Reason: index and name do not match in access
<wtb-jshint.js:48381:26> inlined at <wtb-jshint.js:48446:11>	Reason: index and name do not match in access
<wtb-jshint.js:48381:26> inlined at <wtb-jshint.js:48446:11>	Reason: index and name do not match in access
<wtb-jshint.js:48371:32> inlined at <wtb-jshint.js:48446:11>	Reason: index and name do not match in access
<wtb-jshint.js:48393:26> inlined at <wtb-jshint.js:48446:11>	Reason: wrong map
 
jshint-deopt-loop.png
1.2 MB View Download
Cc: ishell@chromium.org
Status: Started
Simplified example:

====================================
var o = {'ab': 1};

function foo(s) {
  return o[s];
}

var s = 'c' + 'c';
foo(s);
foo(s);
for (var i = 0; i < 10; ++i) {
  %OptimizeFunctionOnNextCall(foo);
  foo(s);
}
====================================
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 20

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/d5c19aa9fc536638b4d3831ba01cf0f1a6a05bbe

commit d5c19aa9fc536638b4d3831ba01cf0f1a6a05bbe
Author: Benedikt Meurer <bmeurer@chromium.org>
Date: Fri Oct 20 12:16:10 2017

[ic] Ensure that we make progress on KeyedLoadIC polymorphic name.

In the special case of KeyedLoadIC, where the key that is passed in is a
Name that is always the same we only checked for identity in both the
stub and the TurboFan case, which works fine for symbols and internalized
strings, but doesn't really work with non-internalized strings, where
the identity check will fail, the runtime will internalize the string,
and the IC will then see the original internalized string again and not
progress in the feedback lattice. This leads to tricky deoptimization
loops in TurboFan and constantly missing ICs.

This adds fixes the stub to always try to internalize strings first
when the identity check fails and then doing the check again. If the
name is not found in the string table we miss, since in that case the
string cannot match the previously recorded feedback name (which is
always a unique name).

In TurboFan we represent this checks with new CheckEqualsSymbol and
CheckEqualsInternalizedString operators, which validate the previously
recorded feedback, and the CheckEqualsInternalizedString operator does
the attempt to internalize the input.

Bug: v8:6936,  v8:6948 ,  v8:6969 
Change-Id: I3f3b4a587c67f00f7c4b60d239eb98a9626fe04a
Reviewed-on: https://chromium-review.googlesource.com/730224
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48784}
[modify] https://crrev.com/d5c19aa9fc536638b4d3831ba01cf0f1a6a05bbe/src/builtins/builtins-definitions.h
[modify] https://crrev.com/d5c19aa9fc536638b4d3831ba01cf0f1a6a05bbe/src/builtins/builtins-ic-gen.cc
[modify] https://crrev.com/d5c19aa9fc536638b4d3831ba01cf0f1a6a05bbe/src/compiler/c-linkage.cc
[modify] https://crrev.com/d5c19aa9fc536638b4d3831ba01cf0f1a6a05bbe/src/compiler/effect-control-linearizer.cc
[modify] https://crrev.com/d5c19aa9fc536638b4d3831ba01cf0f1a6a05bbe/src/compiler/effect-control-linearizer.h
[modify] https://crrev.com/d5c19aa9fc536638b4d3831ba01cf0f1a6a05bbe/src/compiler/js-native-context-specialization.cc
[modify] https://crrev.com/d5c19aa9fc536638b4d3831ba01cf0f1a6a05bbe/src/compiler/js-native-context-specialization.h
[modify] https://crrev.com/d5c19aa9fc536638b4d3831ba01cf0f1a6a05bbe/src/compiler/opcodes.h
[modify] https://crrev.com/d5c19aa9fc536638b4d3831ba01cf0f1a6a05bbe/src/compiler/simplified-lowering.cc
[modify] https://crrev.com/d5c19aa9fc536638b4d3831ba01cf0f1a6a05bbe/src/compiler/simplified-operator.cc
[modify] https://crrev.com/d5c19aa9fc536638b4d3831ba01cf0f1a6a05bbe/src/compiler/simplified-operator.h
[modify] https://crrev.com/d5c19aa9fc536638b4d3831ba01cf0f1a6a05bbe/src/compiler/typed-optimization.cc
[modify] https://crrev.com/d5c19aa9fc536638b4d3831ba01cf0f1a6a05bbe/src/compiler/typed-optimization.h
[modify] https://crrev.com/d5c19aa9fc536638b4d3831ba01cf0f1a6a05bbe/src/compiler/typer.cc
[modify] https://crrev.com/d5c19aa9fc536638b4d3831ba01cf0f1a6a05bbe/src/compiler/verifier.cc
[modify] https://crrev.com/d5c19aa9fc536638b4d3831ba01cf0f1a6a05bbe/src/deoptimize-reason.h
[modify] https://crrev.com/d5c19aa9fc536638b4d3831ba01cf0f1a6a05bbe/src/ic/accessor-assembler.cc
[modify] https://crrev.com/d5c19aa9fc536638b4d3831ba01cf0f1a6a05bbe/src/ic/accessor-assembler.h
[add] https://crrev.com/d5c19aa9fc536638b4d3831ba01cf0f1a6a05bbe/test/mjsunit/regress/regress-6948.js

Status: Fixed

Sign in to add a comment