Bug 137161 - DFG shouldn't insert store barriers when it has it on good authority that we're not storing a cell
Summary: DFG shouldn't insert store barriers when it has it on good authority that we'...
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Filip Pizlo
Depends on: 137340
  Show dependency treegraph
Reported: 2014-09-26 15:47 PDT by Filip Pizlo
Modified: 2014-10-13 17:43 PDT (History)
8 users (show)

See Also:

the patch (5.98 KB, patch)
2014-09-26 15:48 PDT, Filip Pizlo
mhahnenb: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Filip Pizlo 2014-09-26 15:47:53 PDT
Patch forthcoming.
Comment 1 Filip Pizlo 2014-09-26 15:48:57 PDT
Created attachment 238742 [details]
the patch
Comment 2 Mark Hahnenberg 2014-09-26 15:51:03 PDT
Comment on attachment 238742 [details]
the patch

Comment 3 Filip Pizlo 2014-09-26 15:54:06 PDT
Landed in http://trac.webkit.org/changeset/174025
Comment 5 Filip Pizlo 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.
Comment 6 Mark Lam 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.