RESOLVED FIXED 162689
B3 should support trapping memory accesses 
https://bugs.webkit.org/show_bug.cgi?id=162689
Summary B3 should support trapping memory accesses
Filip Pizlo
Reported 2016-09-28 10:50:27 PDT
WebAssembly can be made to run faster if you let virtual memory do some safety checks for you. But that means that B3 needs to know that memory accesses can side-exit in an observable way.
Attachments
work in progress (25.72 KB, patch)
2016-09-30 12:29 PDT, Filip Pizlo 		no flags
DetailsFormatted DiffDiff
the patch (43.55 KB, patch)
2016-09-30 15:21 PDT, Filip Pizlo 		no flags
DetailsFormatted DiffDiff
the patch (43.53 KB, patch)
2016-09-30 16:03 PDT, Filip Pizlo 		ggaren: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Filip Pizlo
Comment 1 2016-09-28 10:50:54 PDT
Keith: which WebAssembly bug should this block?
Filip Pizlo
Comment 2 2016-09-30 12:29:11 PDT
Created attachment 290363 [details] work in progress
Filip Pizlo
Comment 3 2016-09-30 15:21:40 PDT
Created attachment 290399 [details] the patch
Filip Pizlo
Comment 4 2016-09-30 16:03:50 PDT
Created attachment 290402 [details] the patch
Geoffrey Garen
Comment 5 2016-09-30 16:07:18 PDT
Comment on attachment 290402 [details] the patch r=me > Source/JavaScriptCore/b3/B3LowerToAir.cpp:212 > + ArgPromise(ArgPromise&& other) > + { > + swap(other); > + } > + > + ArgPromise& operator=(ArgPromise&& other) > + { > + swap(other); > + return *this; > + } It is generally an anti-pattern for move construction or assignment to swap. If ArgPromise had non-POD data, which benefited from swap, its destructors would run at the wrong time. Since ArgPromise has only POD data, there's no problem, but also no reason to swap. I recommend simple assignment.
Filip Pizlo
Comment 6 2016-09-30 16:13:40 PDT
(In reply to comment #5) > Comment on attachment 290402 [details] > the patch > > r=me > > > Source/JavaScriptCore/b3/B3LowerToAir.cpp:212 > > + ArgPromise(ArgPromise&& other) > > + { > > + swap(other); > > + } > > + > > + ArgPromise& operator=(ArgPromise&& other) > > + { > > + swap(other); > > + return *this; > > + } > > It is generally an anti-pattern for move construction or assignment to swap. > > If ArgPromise had non-POD data, which benefited from swap, its destructors > would run at the wrong time. > > Since ArgPromise has only POD data, there's no problem, but also no reason > to swap. > > I recommend simple assignment. But there is a reason to swap! See: ~ArgPromise() { if (m_wasConsumed) RELEASE_ASSERT(m_wasWrapped); } I get that the fields of ArgPromise look like PODs, but that doesn't matter, since they're used to implement very un-POD-like logic.
Filip Pizlo
Comment 7 2016-09-30 17:11:35 PDT
Landed in https://trac.webkit.org/changeset/206694
Note You need to log in before you can comment on or make changes to this bug.