Bug 137161

Summary:

DFG shouldn't insert store barriers when it has it on good authority that we're not storing a cell

Product:

WebKit

Reporter:

Filip Pizlo <fpizlo>

Component:

JavaScriptCore

Assignee:

Filip Pizlo <fpizlo>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

barraclough, ggaren, mark.lam, mhahnenb, msaboff, oliver, ossy, sam

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Bug Depends on:

137340

Bug Blocks:

Attachments:

Description	Flags
the patch	mhahnenb: review+

Filip Pizlo

Reported 2014-09-26 15:47:53 PDT

Patch forthcoming.

Attachments
the patch (5.98 KB, patch) 2014-09-26 15:48 PDT, Filip Pizlo	mhahnenb: review+	Details Formatted Diff Diff
View All Add attachment proposed patch, testcase, etc.

Filip Pizlo

Comment 1 2014-09-26 15:48:57 PDT

Created attachment 238742 [details] the patch

Mark Hahnenberg

Comment 2 2014-09-26 15:51:03 PDT

Comment on attachment 238742 [details] the patch r=me!

Filip Pizlo

Comment 3 2014-09-26 15:54:06 PDT

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

Csaba Osztrogonác

Comment 4 2014-09-27 10:39:47 PDT

(In reply to comment #3) > Landed in http://trac.webkit.org/changeset/174025 It made 3-4 performance tests crash everywhere: - Apple Mountain Lion: https://build.webkit.org/builders/Apple%20MountainLion%20Release%20%28Perf%29/builds/10126 - Apple Mavericks: https://build.webkit.org/builders/Apple%20Mavericks%20Release%20%28Perf%29/builds/2663 - EFL: https://build.webkit.org/builders/EFL%20Linux%2064-bit%20Release%20WK2%20%28Perf%29/builds/3310

Filip Pizlo

Comment 5 2014-09-27 11:02:21 PDT

(In reply to comment #4) > (In reply to comment #3) > > Landed in http://trac.webkit.org/changeset/174025 > > It made 3-4 performance tests crash everywhere: > - Apple Mountain Lion: https://build.webkit.org/builders/Apple%20MountainLion%20Release%20%28Perf%29/builds/10126 > - Apple Mavericks: https://build.webkit.org/builders/Apple%20Mavericks%20Release%20%28Perf%29/builds/2663 > - EFL: https://build.webkit.org/builders/EFL%20Linux%2064-bit%20Release%20WK2%20%28Perf%29/builds/3310 Ok, I will look. The consensus last us GC people chatted was that the barriers that this patch removed were merely masking the lack of barriers elsewhere. I'd rather fix those other barriers directly rather than rolling this out.

Mark Lam

Comment 6 2014-10-13 17:43:27 PDT

For the record, there was a follow up fix to this patch. The fix was landed in r174121: <http://trac.webkit.org/r174121> by pizlo.

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