Bug 182368

Summary: When BytecodeParser inserts Unreachable after ForceOSRExit it needs to update ArgumentPositions for Flushes it inserts
Product: WebKit Reporter: Saam Barati <saam>
Component: JavaScriptCoreAssignee: Saam Barati <saam>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, commit-queue, fpizlo, ggaren, gskachkov, jfbastien, keith_miller, mark.lam, msaboff, rmorisset, ticaiolima, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
patch
none
patch
none
patch
none
patch none

Saam Barati
Reported 2018-01-31 18:20:15 PST
...
Attachments
patch (33.03 KB, patch)
2018-01-31 18:48 PST, Saam Barati
no flags
patch (10.25 KB, patch)
2018-01-31 18:48 PST, Saam Barati
no flags
patch (10.25 KB, patch)
2018-01-31 19:06 PST, Saam Barati
no flags
patch (10.29 KB, patch)
2018-02-01 11:22 PST, Saam Barati
no flags
Saam Barati
Comment 1 2018-01-31 18:43:27 PST
Saam Barati
Comment 2 2018-01-31 18:48:20 PST
Saam Barati
Comment 3 2018-01-31 18:48:41 PST
Saam Barati
Comment 4 2018-01-31 19:06:46 PST
Saam Barati
Comment 5 2018-02-01 11:22:29 PST
Mark Lam
Comment 6 2018-02-02 13:45:11 PST
Comment on attachment 332895 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=332895&action=review r=me with a suggestion. > Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:6679 > + VirtualRegister argument = operand - (inlineCallFrame ? inlineCallFrame->stackOffset : 0); > + if (argument.isArgument() && !argument.isHeader()) { > + const Vector<ArgumentPosition*>& arguments = m_inlineCallFrameToArgumentPositions.get(inlineCallFrame); > + arguments[argument.toArgument()]->addVariable(variable); > + } If this is only needed when we create a new VAD, maybe you can move this into the body of the if statement above.
Saam Barati
Comment 7 2018-02-02 13:56:44 PST
Comment on attachment 332895 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=332895&action=review >> Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:6679 >> + } > > If this is only needed when we create a new VAD, maybe you can move this into the body of the if statement above. Spoke to Mark offline about this: We want to tell this ArgumentPosition about this VAD regardless of whether we added it or not. From reading other code in the bytecode parser, I believe it's possible that this VAD is from GetLocal only, and the ArgumentPosition would not know about it.
WebKit Commit Bot
Comment 8 2018-02-02 14:55:33 PST
Comment on attachment 332895 [details] patch Clearing flags on attachment: 332895 Committed r228031: <https://trac.webkit.org/changeset/228031>
WebKit Commit Bot
Comment 9 2018-02-02 14:55:35 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.