New issue
Advanced search Search tips
Starred by 1 user
Status: Fixed
Owner:
Closed: Oct 21
Cc:
Components:
HW: All
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
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