AuroraMiddleware/v8
1
0
Fork 0
Code Issues 2 Pull Requests Releases Wiki Activity

[turbofan] Properly handle smis in monomorphic loads/stores.

Browse Source 
When lowering a monomorphic load/store, where multiple receiver maps
have been recorded, but the action to be performed is the same (i.e.
yielding undefined because the property is not found), TurboFan used
to ignore the Smi case, leading to a pretty terrible deoptimization
loop, as the LOAD_IC/STORE_IC properly recorded that state and thus
didn't change it's state.

Fixing this issue gives a 18-20% boost on the prettier test of the
web-tooling-benchmark, which was suffering a lot from this problem.

Bug: v8:6936, v8:6991
Change-Id: Id208ec7129a7f6b190d989bda31f936040393226
Reviewed-on: https://chromium-review.googlesource.com/735342
Reviewed-by: Michael Stanton <mvstanton@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48865}
This commit is contained in:
Benedikt Meurer 2017-10-24 10:39:25 +02:00 committed by Commit Bot
parent bb6bee3255
commit a9da0ce735
2 changed files with 43 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
src/compiler/js-native-context-specialization.cc
View File

@ -741,10 +741,32 @@ Reduction JSNativeContextSpecialization::ReduceNamedAccess(
&receiver, &effect, control) &&
!access_builder.TryBuildNumberCheck(access_info.receiver_maps(),
&receiver, &effect, control)) {
receiver =
access_builder.BuildCheckHeapObject(receiver, &effect, control);
access_builder.BuildCheckMaps(receiver, &effect, control,
access_info.receiver_maps());
if (HasNumberMaps(access_info.receiver_maps())) {
// We need to also let Smi {receiver}s through in this case, so
// we construct a diamond, guarded by the Sminess of the {receiver}
// and if {receiver} is not a Smi just emit a sequence of map checks.
Node* check = graph()->NewNode(simplified()->ObjectIsSmi(), receiver);
Node* branch = graph()->NewNode(common()->Branch(), check, control);
Node* if_true = graph()->NewNode(common()->IfTrue(), branch);
Node* etrue = effect;
Node* if_false = graph()->NewNode(common()->IfFalse(), branch);
Node* efalse = effect;
{
access_builder.BuildCheckMaps(receiver, &efalse, if_false,
access_info.receiver_maps());
}
control = graph()->NewNode(common()->Merge(2), if_true, if_false);
effect =
graph()->NewNode(common()->EffectPhi(2), etrue, efalse, control);
} else {
receiver =
access_builder.BuildCheckHeapObject(receiver, &effect, control);
access_builder.BuildCheckMaps(receiver, &effect, control,
access_info.receiver_maps());
}
}
// Generate the actual property access.

17
test/mjsunit/regress/regress-6991.js Normal file
View File

@ -0,0 +1,17 @@
// Copyright 2017 the V8 project authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// Flags: --allow-natives-syntax --opt
function foo(o) { return o.x; }
assertEquals(undefined, foo({}));
assertEquals(undefined, foo(1));
assertEquals(undefined, foo({}));
assertEquals(undefined, foo(1));
%OptimizeFunctionOnNextCall(foo);
assertEquals(undefined, foo({}));
assertOptimized(foo);
assertEquals(undefined, foo(1));
assertOptimized(foo);