Bug 87205

Summary: DFG should keep captured variables alive until the (inline) return.
Product: WebKit Reporter: Filip Pizlo <fpizlo>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: kangil.han
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 85945    
Bug Blocks: 87813    
Attachments:
Description Flags
work in progress
none
it's starting to work
none
more
none
even more
none
the patch barraclough: review+

Description Filip Pizlo 2012-05-22 21:22:25 PDT
Otherwise OSR exit may decided to set them to Undefined.
Comment 1 Filip Pizlo 2012-05-22 23:57:50 PDT
Currently a variable may appear to be dead at a basic block boundary even though it is captured. As a hack, we can peek at the first node of the successor block(s) and check what variables are captured at that node's code origin. But that's just disgusting and wrong.

The right solution is to change how captured variables are flushed. Currently we flush SetLocals by planting a Flush node right after them:

a: SetLocal(thingy)
b: Flush(@a)

But we could instead have a flush right before each SetLocal:

a: Flush(...)
b: SetLocal(thingy)

As well as a Flush for all captured variables at the return site. That will effectively keep variables alive through the whole span of code in which they are captured.
Comment 2 Filip Pizlo 2012-05-28 23:37:27 PDT
Created attachment 144458 [details]
work in progress
Comment 3 Filip Pizlo 2012-05-29 00:09:17 PDT
Created attachment 144464 [details]
it's starting to work
Comment 4 Filip Pizlo 2012-05-29 14:06:28 PDT
Created attachment 144616 [details]
more

Still making sure that I've dotted all of my t's.
Comment 5 Filip Pizlo 2012-05-29 15:06:25 PDT
Created attachment 144629 [details]
even more

It's starting to pass tests.
Comment 6 Filip Pizlo 2012-05-29 16:14:38 PDT
Created attachment 144638 [details]
the patch
Comment 7 Filip Pizlo 2012-05-29 16:44:05 PDT
Landed in http://trac.webkit.org/changeset/118858
Comment 8 Kangil Han 2012-07-31 07:34:00 PDT
Just for your information, I've got a build warning on my laptop as below. :-)

[  5%] Building CXX object Source/JavaScriptCore/CMakeFiles/javascriptcore_efl.dir/dfg/DFGCapabilities.cpp.o
/home/kangilhan/dev/WebKit/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp: In member function ‘JSC::DFG::ArgumentPosition* JSC::DFG::ByteCodeParser::findArgumentPositionForLocal(int)’:
/home/kangilhan/dev/WebKit/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:361:73: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]