141624 – DFG::PutStackSinkingPhase should eliminate GetStacks that have an obviously known source, and emit GetStacks when the stack's value is needed and none is deferred

Bug 141624 - DFG::PutStackSinkingPhase should eliminate GetStacks that have an obviously known source, and emit GetStacks when the stack's value is needed and none is deferred

Summary: DFG::PutStackSinkingPhase should eliminate GetStacks that have an obviously k...

Status:	RESOLVED FIXED

Alias:	None

Product:	WebKit
Classification:	Unclassified
Component:	JavaScriptCore (show other bugs)
Version:	528+ (Nightly build)
Hardware:	All All

Importance:	P2 Normal
Assignee:	Filip Pizlo

URL:
Keywords:

Depends on:	142695 142697
Blocks:	141174
	Show dependency tree / graph

Reported:	2015-02-15 14:41 PST by Filip Pizlo
Modified:	2015-03-16 10:39 PDT (History)
CC List:	13 users (show)

See Also:

Attachments
the patch (4.57 KB, patch) 2015-03-13 13:01 PDT, Filip Pizlo	oliver: review+	Details \| Formatted Diff \| Diff
the patch (12.65 KB, patch) 2015-03-15 20:54 PDT, Filip Pizlo	ggaren: review+	Details \| Formatted Diff \| Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.

Description Filip Pizlo 2015-02-15 14:41:02 PST

Currently if we have a PutLocal/GetLocal pair, then we will sink the PutLocal to the GetLocal.  Instead, we should eliminate the GetLocal entirely.

Probably this doesn't arise too much because we would have done some CSE, but it would be best if we just did the right thing here.

Comment 1 Filip Pizlo 2015-03-13 13:01:51 PDT

Created attachment 248599 [details]
the patch

Comment 2 Filip Pizlo 2015-03-13 13:18:57 PDT

Landed in http://trac.webkit.org/changeset/181487

Comment 3 Simon Fraser (smfr) 2015-03-14 09:01:22 PDT

This is causing a jsc test failure:

https://build.webkit.org/builders/Apple%20Mavericks%20Release%20WK2%20%28Perf%29

Running Speedometer/Full.html (144 of 144)
error: Speedometer/Full.html
1   0x108f858a3 JSC::DFG::(anonymous namespace)::PutStackSinkingPhase::run()
2   0x108f7c5f4 JSC::DFG::performPutStackSinking(JSC::DFG::Graph&)
3   0x108fd5cf9 JSC::DFG::Plan::compileInThreadImpl(JSC::DFG::LongLivedState&)
4   0x108fd5601 JSC::DFG::Plan::compileInThread(JSC::DFG::LongLivedState&, JSC::DFG::ThreadData*)
5   0x1090601d3 JSC::DFG::Worklist::runThread(JSC::DFG::ThreadData*)
6   0x1093c5bd3 WTF::threadEntryPoint(void*)
7   0x1093c608f WTF::wtfThreadEntryPoint(void*)
8   0x7fff85ad7899 _pthread_body
9   0x7fff85ad772a _pthread_struct_init
10  0x7fff85adbfc9 thread_start

Comment 4 WebKit Commit Bot 2015-03-14 10:43:13 PDT

Re-opened since this is blocked by bug 142695

Comment 5 Filip Pizlo 2015-03-15 18:59:00 PDT

Comment on attachment 248599 [details]
the patch

This patch was broken, and it revealed some other issues in this phase.  I'll have a new patch shortly.

Comment 6 Filip Pizlo 2015-03-15 20:54:19 PDT

Created attachment 248700 [details]
the patch

Comment 7 Geoffrey Garen 2015-03-16 10:30:02 PDT

Comment on attachment 248700 [details]
the patch

r=me

Comment 8 Filip Pizlo 2015-03-16 10:39:54 PDT

Landed in http://trac.webkit.org/changeset/181563