|Summary:||FTL keepAlive()'s patchpoint should also declare that it reads HeapRange::top().|
|Product:||WebKit||Reporter:||Mark Lam <mark.lam>|
|Severity:||Normal||CC:||fpizlo, justin_michaud, keith_miller, msaboff, rmorisset, sbarati, tzagallo, webkit-bug-importer, ysuzuki|
|Version:||WebKit Nightly Build|
Description Mark Lam 2019-06-27 15:13:20 PDT
The main purpose of keepAlive() is to communicate to B3 that an LValue needs to be kept alive pass the last opportunity for a GC. The only way we can get a GC is via a function call. Hence, what keepAlive() really needs to communicate is that the LValue needs to be kept alive pass the last function call. Function calls read and write HeapRange::top(). Currently, B3 does not shuffle writes. Hence, simply inserting the keepAlive() after the calls that can GC is sufficient. But to be strictly correct, keepAlive() should also declare that it reads HeapRange::top(). This will guarantee that the keepAlive patchpoint won't ever be moved before the function call should B3 gain the ability to shuffle writes in the future.
Comment 2 Yusuke Suzuki 2019-06-27 15:23:42 PDT
Comment on attachment 373057 [details] proposed patch. r=me
Comment 3 Filip Pizlo 2019-06-27 16:00:05 PDT
Comment on attachment 373057 [details] proposed patch. R=me too.
Comment 4 Mark Lam 2019-06-27 17:27:23 PDT
Thanks for the reviews. Landed in r246910: <http://trac.webkit.org/r246910>.