RESOLVED FIXED172306
B3::Value::effects() says that having a fence range implies the fence bit, but on x86_64 we lower loadAcq/storeRel to load/store so the store-before-load fence bit orderings won't be honored 
https://bugs.webkit.org/show_bug.cgi?id=172306
Summary B3::Value::effects() says that having a fence range implies the fence bit, bu...
Filip Pizlo
Reported 2017-05-18 12:51:41 PDT
As the title says.
Attachments
possible patch (22.72 KB, patch)
2017-05-18 15:51 PDT, Filip Pizlo 		no flags
DetailsFormatted DiffDiff
the patch (32.61 KB, patch)
2017-05-18 16:48 PDT, Filip Pizlo 		no flags
DetailsFormatted DiffDiff
the patch (32.60 KB, patch)
2017-05-18 17:29 PDT, Filip Pizlo 		msaboff: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Filip Pizlo
Comment 1 2017-05-18 12:52:44 PDT
I think that this means that MacroAssembler::storeRel on x86 should emit a xchg.
Filip Pizlo
Comment 2 2017-05-18 12:56:55 PDT
Seems like MacroAssembler even having methods like loadAcq and storeRel is super confusing. B3::LowerToAir should be making a deliberate choice about what acq/rel mean for x86, and I think that the only right interpretation is that rel means xchg.
Radar WebKit Bug Importer
Comment 3 2017-05-18 13:21:27 PDT
<rdar://problem/32279715>
Filip Pizlo
Comment 4 2017-05-18 15:51:10 PDT
Created attachment 310565 [details] possible patch
Filip Pizlo
Comment 5 2017-05-18 16:48:45 PDT
Created attachment 310575 [details] the patch
Filip Pizlo
Comment 6 2017-05-18 17:29:35 PDT
Created attachment 310578 [details] the patch
Michael Saboff
Comment 7 2017-05-18 19:58:02 PDT
Comment on attachment 310578 [details] the patch r=me
Filip Pizlo
Comment 8 2017-05-19 08:49:01 PDT
Landed in http://trac.webkit.org/changeset/217127/webkit
Note You need to log in before you can comment on or make changes to this bug.