Bug 162417

Summary: Fences on x86 should be a lot cheaper
Product: WebKit Reporter: Filip Pizlo <fpizlo>
Component: Web Template FrameworkAssignee: Filip Pizlo <fpizlo>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, benjamin, cdumez, cmarcelo, commit-queue, dbates, ggaren, jfbastien, keith_miller, mark.lam, msaboff, saam
Priority: P2    
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 162316    
Attachments:
Description Flags
the patch mark.lam: review+

Filip Pizlo
Reported 2016-09-22 10:50:13 PDT
Patch forthcoming.
Attachments
the patch (6.27 KB, patch)
2016-09-22 13:55 PDT, Filip Pizlo 		mark.lam: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Filip Pizlo
Comment 1 2016-09-22 13:55:23 PDT
Created attachment 289592 [details] the patch
Mark Lam
Comment 2 2016-09-22 14:06:11 PDT
Comment on attachment 289592 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=289592&action=review r=me > Source/JavaScriptCore/ChangeLog:22 > + compiles to Air MemoryFence, which is just MacroAssembler::memoryFene(), this also changes /Fene/Fence/.
Geoffrey Garen
Comment 3 2016-09-22 14:07:01 PDT
r=me Good fences make good neighbors.
Filip Pizlo
Comment 4 2016-09-22 14:12:46 PDT
(In reply to comment #2) > Comment on attachment 289592 [details] > the patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=289592&action=review > > r=me > > > Source/JavaScriptCore/ChangeLog:22 > > + compiles to Air MemoryFence, which is just MacroAssembler::memoryFene(), this also changes > > /Fene/Fence/. Fixed.
Filip Pizlo
Comment 5 2016-09-22 14:15:15 PDT
Landed in https://trac.webkit.org/changeset/206274
JF Bastien
Comment 6 2016-09-22 15:45:43 PDT
Very cool! I forgot to say when I suggested this: AFAIK this doesn't order non-temporals. You probably want to do lfence / sfence around Its anyways, so the point is moot. You may just have happened to get fencing for Its before because of the mfence :-)
Filip Pizlo
Comment 7 2016-09-22 17:11:51 PDT
(In reply to comment #6) > Very cool! > > I forgot to say when I suggested this: AFAIK this doesn't order > non-temporals. You probably want to do lfence / sfence around Its anyways, > so the point is moot. You may just have happened to get fencing for Its > before because of the mfence :-) Fortunately, I don't think we use non-temporals anywhere. I had some code where I tried to use them at one point for memset, but that was when I found out that rep stosl is the bomb dot com.
Note You need to log in before you can comment on or make changes to this bug.