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
Constantly missing on KeyedLoadIC
Project Member Reported by bmeu...@chromium.org, Oct 20 Back to list
This is probably related to  crbug.com/v8/6948 , likely the same underlying issue.

In the buble test case of the web-tooling-benchmark we see a lot of 1->1 misses on certain KeyedLoadICs, for example

KeyedLoadIC (1->1) at ~contains wtb-buble.js:39264:27 arguments (map 0x18f7f63860d1)

happens around 1316 times in a usual run, where we clearly don't learn in the IC. This seems to be related to ThinStrings and not handling them in the PolymorphicName case.

Since TurboFan relies on the IC feedback, we also unnecessarily optimize and deoptimize functions inlining contains quite often.
 
Project Member Comment 1 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