RESOLVED FIXED 162417
Fences on x86 should be a lot cheaper
https://bugs.webkit.org/show_bug.cgi?id=162417
Summary Fences on x86 should be a lot cheaper
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+
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
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.