WebKit Bugzilla
Attachment 368594 Details for
Bug 197405
: [JSC] Inlining Getter/Setter should care availability of ad-hocly constructed frame
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
bug-197405-20190430130829.patch (text/plain), 6.01 KB, created by
Yusuke Suzuki
on 2019-04-30 13:08:30 PDT
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
Yusuke Suzuki
Created:
2019-04-30 13:08:30 PDT
Size:
6.01 KB
patch
obsolete
>Subversion Revision: 244763 >diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog >index 5778ff4fbe3694bc0bc14140680e34a04ed56f5b..79d73b20421946c138f352619d1c107af94b1ed9 100644 >--- a/Source/JavaScriptCore/ChangeLog >+++ b/Source/JavaScriptCore/ChangeLog >@@ -1,3 +1,13 @@ >+2019-04-30 Yusuke Suzuki <ysuzuki@apple.com> >+ >+ [JSC] Inlining Getter/Setter should care availability of ad-hocly constructed frame >+ https://bugs.webkit.org/show_bug.cgi?id=197405 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ * dfg/DFGByteCodeParser.cpp: >+ (JSC::DFG::ByteCodeParser::inlineCall): >+ > 2019-04-29 Yusuke Suzuki <ysuzuki@apple.com> > > Unreivewed, fix FTL implementation of r244760 >diff --git a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp >index be99551e91d58697dcf2754b26982ca48bc775f2..c03bac09bb75068b380f1b901bfef33ecd20c682 100644 >--- a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp >+++ b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp >@@ -1608,6 +1608,48 @@ void ByteCodeParser::inlineCall(Node* callTargetNode, VirtualRegister result, Ca > calleeVariable->mergeShouldNeverUnbox(true); > } > >+ switch (kind) { >+ case InlineCallFrame::GetterCall: >+ case InlineCallFrame::SetterCall: { >+ // When inlining getter and setter calls, we setup the stack frame which does not appear in the bytecode. >+ // But inlining call can have switch on executable. So we could have a graph like this. >+ // >+ // BB#0 >+ // ... >+ // 19:<!0:-> MovHint(Check:Untyped:@18, MustGen, loc4, W:SideState, ClobbersExit, bc#1, ExitValid) >+ // 20:< 1:-> SetLocal(Check:Untyped:@18, loc4(J~/FlushedJSValue), W:Stack(-5), bc#1, exit: bc#3, ExitValid) predicting None >+ // 21:<!0:-> MovHint(Check:Untyped:@18, MustGen, loc5, W:SideState, ClobbersExit, bc#3, ExitValid) >+ // 22:< 1:-> SetLocal(Check:Untyped:@18, loc5(K~/FlushedJSValue), W:Stack(-6), bc#3, exit: bc#6, ExitValid) predicting None >+ // ... >+ // 30:< 1:-> GetSetter(Check:Untyped:@29, JS|PureInt, R:GetterSetter_setter, Exits, bc#7, ExitValid) >+ // ... >+ // 37:< 1:-> GetExecutable(Check:Untyped:@30, JS|PureInt, Exits, bc#7, ExitValid) >+ // ... >+ // 41:<!0:-> Switch(Check:Untyped:@37, MustGen, SwitchCell, Weak:Cell: 0x1057c3680 (%Ei:FunctionExecutable), StructureID: 18859:#2, default:#3, W:SideState, Exits, bc#7, ExitValid) >+ // >+ // BB#2 >+ // 42:<!0:-> GetLocal(JS|MustGen|PureInt, loc12(O~/FlushedJSValue), R:Stack(-13), bc#7, ExitValid) predicting None >+ // 43:<!0:-> MovHint(Check:Untyped:@42, MustGen, loc12, W:SideState, ClobbersExit, bc#7, ExitValid) >+ // 44:< 1:-> SetLocal(Check:Untyped:@42, loc12(P!/FlushedJSValue), W:Stack(-13), bc#7, ExitInvalid) predicting None >+ // --> f#DMIaZz:<0x1057a0390, bc#7, SetterCall, closure call, numArgs+this = 2, numFixup = 0, stackOffset = -16 (loc0 maps to loc16)> >+ // 45:<!0:-> ExitOK(MustGen, R:Stack(-13), W:SideState, bc#0, ExitValid) >+ // ... >+ // <HERE> >+ // >+ // BB#2's first node is @42, which origin is bc#7 of the caller (not callee f#DMIaZz). So OSR availability phase can prune @19 and @21 liveness, because these stack values are set up >+ // adhocly by DFG and it won't be shown in the bytecode. If <HERE> causes OSR exit, we fail to find loc4 and loc5. Here, we emit MovHint in BB#2 to tell f#DMIaZz in BB#2 the frame layout, >+ // as if we are doing similarly for arity fixup. Since these locals are already flushed to the appropriate place, we do not need to emit SetLocal later. >+ for (int index = 0; index < argumentCountIncludingThis; ++index) { >+ Node* value = get(virtualRegisterForArgument(index, registerOffset)); >+ VirtualRegister argumentToSet = m_inlineStackTop->remapOperand(virtualRegisterForArgument(index, registerOffsetAfterFixup)); >+ addToGraph(MovHint, OpInfo(argumentToSet.offset()), value); >+ } >+ break; >+ } >+ default: >+ break; >+ } >+ > if (arityFixupCount) { > // Note: we do arity fixup in two phases: > // 1. We get all the values we need and MovHint them to the expected locals. >diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog >index aafd97331929c0a502e20e66cc2677cfceda3ed0..874d903c9bc49d9ac7c5533fc13a8e2c6a65a45c 100644 >--- a/JSTests/ChangeLog >+++ b/JSTests/ChangeLog >@@ -1,3 +1,16 @@ >+2019-04-30 Yusuke Suzuki <ysuzuki@apple.com> >+ >+ [JSC] Inlining Getter/Setter should care availability of ad-hocly constructed frame >+ https://bugs.webkit.org/show_bug.cgi?id=197405 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ * stress/getter-setter-inlining-should-emit-movhint.js: Added. >+ (foo): >+ (test): >+ (i.o.get f): >+ (i.o.set f): >+ > 2019-04-29 Yusuke Suzuki <ysuzuki@apple.com> > > normalizeMapKey should normalize NaN to one PureNaN bit pattern to make MapHash same >diff --git a/JSTests/stress/getter-setter-inlining-should-emit-movhint.js b/JSTests/stress/getter-setter-inlining-should-emit-movhint.js >new file mode 100644 >index 0000000000000000000000000000000000000000..616fdf06d7c48834be19606f002e911faa870ad2 >--- /dev/null >+++ b/JSTests/stress/getter-setter-inlining-should-emit-movhint.js >@@ -0,0 +1,30 @@ >+//@ runDefault("--useRandomizingFuzzerAgent=1", "--usePolymorphicCallInliningForNonStubStatus=1", "--seedOfRandomizingFuzzerAgent=2896922505", "--useLLInt=0") >+function foo(o) { >+ o.f = 0; >+ return o.f; >+} >+noInline(foo); >+ >+let counter = 0; >+ >+function test(o, value) { >+ var result = foo(o); >+ if (result < value) >+ throw new Error(result); >+ if (counter < value) >+ throw new Error(counter); >+ Array.of(arguments); >+} >+ >+for (var i = 0; i < 100000; ++i) { >+ var o = { >+ get f() { >+ return o >+ }, >+ set f(v) { >+ counter++; >+ this.z = 0; >+ } >+ }; >+ test(o, i, i); >+}
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 197405
:
368542
|
368594
|
368596
|
368622
|
368627
|
368643
|
368644
|
368649
|
368650
|
368744