125964 2013-12-18 17:29:19 -0800 CStack Branch: Stop threading callFrameRegister through LLIntSlowCalls 2013-12-18 22:34:45 -0800 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 1 msaboff msaboff ggaren oldest_to_newest 961195 0 msaboff 2013-12-18 17:29:19 -0800 The LLint always passes the callFrameRegister to LLInt slow patch calls as the "exec" argument. The slow calls always return an exec value, usually the one passed in. There is no need for this now that we are using the CPU's call frame register. In some cases it is even wrong. Instead we should only use the second return value when we have a real value to return, like the execCallee for a llint_slow_path_call(). 961200 1 219595 msaboff 2013-12-18 17:44:17 -0800 Created attachment 219595 Patch 961211 2 219595 ggaren 2013-12-18 18:05:38 -0800 Comment on attachment 219595 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=219595&action=review r=me > Source/JavaScriptCore/llint/LowLevelInterpreter.asm:804 > callSlowPath(_llint_slow_path_size_and_alloc_frame_for_varargs) > branchIfException(_llint_throw_from_slow_path_trampoline) > - loadp CodeBlock[cfr], t0 > - loadp CodeBlock::m_vm[t0], t0 > - loadp VM::newCallFrameReturnValue[t0], t0 > - move t0, sp > + # calleeFrame in t1 > + move t1, sp Is it valid for _llint_slow_path_size_and_alloc_frame_for_varargs to allocate calleeFrame like this? 961212 3 msaboff 2013-12-18 18:09:26 -0800 (In reply to comment #2) > (From update of attachment 219595 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=219595&action=review > > r=me > > > Source/JavaScriptCore/llint/LowLevelInterpreter.asm:804 > > callSlowPath(_llint_slow_path_size_and_alloc_frame_for_varargs) > > branchIfException(_llint_throw_from_slow_path_trampoline) > > - loadp CodeBlock[cfr], t0 > > - loadp CodeBlock::m_vm[t0], t0 > > - loadp VM::newCallFrameReturnValue[t0], t0 > > - move t0, sp > > + # calleeFrame in t1 > > + move t1, sp > > Is it valid for _llint_slow_path_size_and_alloc_frame_for_varargs to allocate calleeFrame like this? Yes it is, because all it is doing is address arithmetic, i.e. where the frame should go. It also does the stack check. However it never writes anything into the new frame header. 961215 4 msaboff 2013-12-18 18:18:44 -0800 Committed r160815: <http://trac.webkit.org/changeset/160815> 961219 5 ggaren 2013-12-18 18:23:59 -0800 I see. Can we rename _llint_slow_path_size_and_alloc_frame_for_varargs to _llint_slow_path_size_frame_for_varargs, and similar names, if any? 961278 6 msaboff 2013-12-18 22:34:45 -0800 (In reply to comment #5) > I see. Can we rename _llint_slow_path_size_and_alloc_frame_for_varargs to _llint_slow_path_size_frame_for_varargs, and similar names, if any? WIll do <https://bugs.webkit.org/show_bug.cgi?id=125980> 219595 2013-12-18 17:44:17 -0800 2013-12-18 18:05:38 -0800 Patch 125964.patch text/plain 5688 msaboff SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMTYwODEyKQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDIzIEBA CisyMDEzLTEyLTE4ICBNaWNoYWVsIFNhYm9mZiAgPG1zYWJvZmZAYXBwbGUuY29tPgorCisgICAg ICAgIENTdGFjayBCcmFuY2g6IFN0b3AgdGhyZWFkaW5nIGNhbGxGcmFtZVJlZ2lzdGVyIHRocm91 Z2ggTExJbnRTbG93Q2FsbHMKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19i dWcuY2dpP2lkPTEyNTk2NAorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgor CisgICAgICAgIFJlbW92ZWQgdGhlIGdlbmVyYWwgcmVzdG9yaW5nIG9mIHRoZSBjYWxsIGZyYW1l IHJlZ2lzdGVycyAoY2ZyKSBhZnRlciB0aGUgcmV0dXJuCisgICAgICAgIGZyb20gYSBMTEludCBz bG93IGNhbGxzLiAgSW4gbW9zdCBjYXNlcywgdGhlICJleGVjIiB2YWx1ZSBvZiB0aGUgTExJbnQg c2xvdworICAgICAgICBwYXRoIHJldHVybiBwYWlyIGlzIHNldCB0byAwLiAgRm9yIGxsaW50X3Ns b3dfcGF0aF9jYWxsL2NvbnN0cnVjdCBhbmQgCisgICAgICAgIGxsaW50X3Nsb3dfcGF0aF9zaXpl X2FuZF9hbGxvY19mcmFtZV9mb3JfdmFyYXJncyB0aGUgZXhlY0NhbGxlZSBpcyByZXR1cm5lZCBp bgorICAgICAgICB0aGUgImV4ZWMiIHZhbHVlLiAgV2hlbiBhbiBleGNlcHRpb24gaGFwcGVucyBp biBhIHNsb3cgcGF0aCBoYW5kbGVyIHdlIHJldHVybiAwLAorICAgICAgICBiZWNhdXNlIHRoZSBs bGludF90aHJvd19mcm9tX3Nsb3dfcGF0aF90cmFtcG9saW5lIHBhdGggd2lsbCBwcm9wZXJseSB1 bndpbmQgYnkKKyAgICAgICAgbWFraW5nIGEgc2xvdyBjYWxsIHRoYXQgd2lsbCBpbnZva2UgZ2Vu ZXJpY1Vud2luZCgpIHRvIGRvIHRoZSByZWFsIHVud2luZGluZy4KKworICAgICAgICAqIGxsaW50 L0xMSW50U2xvd1BhdGhzLmNwcDoKKyAgICAgICAgKEpTQzo6TExJbnQ6OkxMSU5UX1NMT1dfUEFU SF9ERUNMKToKKyAgICAgICAgKiBsbGludC9Mb3dMZXZlbEludGVycHJldGVyLmFzbToKKyAgICAg ICAgKiBsbGludC9Mb3dMZXZlbEludGVycHJldGVyNjQuYXNtOgorCiAyMDEzLTEyLTE4ICBNYXJr IExhbSAgPG1hcmsubGFtQGFwcGxlLmNvbT4KIAogICAgICAgICBDU3RhY2s6IEZpeCBMTElOVCB0 byBiYXNlbGluZSBKSVQgT1NSLgpJbmRleDogU291cmNlL0phdmFTY3JpcHRDb3JlL2xsaW50L0xM SW50U2xvd1BhdGhzLmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvSmF2YVNjcmlwdENvcmUvbGxp bnQvTExJbnRTbG93UGF0aHMuY3BwCShyZXZpc2lvbiAxNjA3ODUpCisrKyBTb3VyY2UvSmF2YVNj cmlwdENvcmUvbGxpbnQvTExJbnRTbG93UGF0aHMuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC04Miw3 ICs4Miw3IEBAIG5hbWVzcGFjZSBKU0MgeyBuYW1lc3BhY2UgTExJbnQgewogICAgICAgICByZXR1 cm4gZW5jb2RlUmVzdWx0KGZpcnN0LCBzZWNvbmQpOyAgICAgICAgXAogICAgIH0gd2hpbGUgKGZh bHNlKQogCi0jZGVmaW5lIExMSU5UX0VORF9JTVBMKCkgTExJTlRfUkVUVVJOX1RXTyhwYywgZXhl YykKKyNkZWZpbmUgTExJTlRfRU5EX0lNUEwoKSBMTElOVF9SRVRVUk5fVFdPKHBjLCAwKQogCiAj ZGVmaW5lIExMSU5UX1RIUk9XKGV4Y2VwdGlvblRvVGhyb3cpIGRvIHsgICAgICAgICAgICAgICAg ICAgICAgICBcCiAgICAgICAgIHZtLnRocm93RXhjZXB0aW9uKGV4ZWMsIGV4Y2VwdGlvblRvVGhy b3cpOyAgICAgICAgICAgICAgICBcCkBAIC0xNDUsMjIgKzE0NSwyOCBAQCBuYW1lc3BhY2UgSlND IHsgbmFtZXNwYWNlIExMSW50IHsKICNkZWZpbmUgTExJTlRfQ0FMTF9USFJPVyhleGVjLCBleGNl cHRpb25Ub1Rocm93KSBkbyB7ICAgICAgICAgICAgICAgICAgIFwKICAgICAgICAgRXhlY1N0YXRl KiBfX2N0X2V4ZWMgPSAoZXhlYyk7ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIFwK ICAgICAgICAgdm0udGhyb3dFeGNlcHRpb24oX19jdF9leGVjLCBleGNlcHRpb25Ub1Rocm93KTsg ICAgICAgICAgICAgICAgIFwKLSAgICAgICAgTExJTlRfQ0FMTF9FTkRfSU1QTChfX2N0X2V4ZWMs IGNhbGxUb1Rocm93KF9fY3RfZXhlYykpOyAgICAgICAgIFwKKyAgICAgICAgTExJTlRfQ0FMTF9F TkRfSU1QTCgwLCBjYWxsVG9UaHJvdyhfX2N0X2V4ZWMpKTsgICAgICAgICAgICAgICAgIFwKICAg ICB9IHdoaWxlIChmYWxzZSkKIAogI2RlZmluZSBMTElOVF9DQUxMX0NIRUNLX0VYQ0VQVElPTihl eGVjKSBkbyB7ICAgICAgICAgICAgICAgICAgICAgICAgICAgXAogICAgICAgICBFeGVjU3RhdGUq IF9fY2NlX2V4ZWMgPSAoZXhlYyk7ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgXAog ICAgICAgICBpZiAoVU5MSUtFTFkodm0uZXhjZXB0aW9uKCkpKSAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgXAotICAgICAgICAgICAgTExJTlRfQ0FMTF9FTkRfSU1QTChfX2NjZV9l eGVjLCBjYWxsVG9UaHJvdyhfX2NjZV9leGVjKSk7ICAgXAorICAgICAgICAgICAgTExJTlRfQ0FM TF9FTkRfSU1QTCgwLCBjYWxsVG9UaHJvdyhfX2NjZV9leGVjKSk7ICAgICAgICAgICAgXAogICAg IH0gd2hpbGUgKGZhbHNlKQogCiAjZGVmaW5lIExMSU5UX0NBTExfUkVUVVJOKGV4ZWMsIGNhbGxU YXJnZXQpIGRvIHsgICAgICAgICAgICAgICAgICAgICAgICBcCiAgICAgICAgIEV4ZWNTdGF0ZSog X19jcl9leGVjID0gKGV4ZWMpOyAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBcCiAg ICAgICAgIHZvaWQqIF9fY3JfY2FsbFRhcmdldCA9IChjYWxsVGFyZ2V0KTsgICAgICAgICAgICAg ICAgICAgICAgICAgICBcCi0gICAgICAgIExMSU5UX0NBTExfQ0hFQ0tfRVhDRVBUSU9OKF9fY3Jf ZXhlYyk7ICAgICAgICAgICBcCisgICAgICAgIExMSU5UX0NBTExfQ0hFQ0tfRVhDRVBUSU9OKF9f Y3JfZXhlYyk7ICAgICAgICAgICAgICAgICAgICAgICAgICBcCiAgICAgICAgIExMSU5UX0NBTExf RU5EX0lNUEwoX19jcl9leGVjLCBfX2NyX2NhbGxUYXJnZXQpOyAgICAgICAgICAgICAgICBcCiAg ICAgfSB3aGlsZSAoZmFsc2UpCiAKKyNkZWZpbmUgTExJTlRfUkVUVVJOX0NBTExFRV9GUkFNRShl eGVjQ2FsbGVlKSBkbyB7ICAgICAgICAgICAgICAgICAgICAgIFwKKyAgICAgICAgRXhlY1N0YXRl KiBfX3JjZl9leGVjID0gKGV4ZWNDYWxsZWUpOyAgICAgICAgICAgICAgICAgICAgICAgICAgIFwK KyAgICAgICAgTExJTlRfUkVUVVJOX1RXTyhwYywgX19yY2ZfZXhlYyk7ICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgIFwKKyAgICB9IHdoaWxlIChmYWxzZSkKKyAgICAKKwogZXh0ZXJuICJD IiBTbG93UGF0aFJldHVyblR5cGUgbGxpbnRfdHJhY2Vfb3BlcmFuZChFeGVjU3RhdGUqIGV4ZWMs IEluc3RydWN0aW9uKiBwYywgaW50IGZyb21XaGVyZSwgaW50IG9wZXJhbmQpCiB7CiAgICAgTExJ TlRfQkVHSU4oKTsKQEAgLTExMTcsNyArMTEyMyw3IEBAIExMSU5UX1NMT1dfUEFUSF9ERUNMKHNs b3dfcGF0aF9zaXplX2FuZF8KICAgICAKICAgICB2bS5uZXdDYWxsRnJhbWVSZXR1cm5WYWx1ZSA9 IGV4ZWNDYWxsZWU7CiAKLSAgICBMTElOVF9FTkQoKTsKKyAgICBMTElOVF9SRVRVUk5fQ0FMTEVF X0ZSQU1FKGV4ZWNDYWxsZWUpOwogfQogCiBMTElOVF9TTE9XX1BBVEhfREVDTChzbG93X3BhdGhf Y2FsbF92YXJhcmdzKQpJbmRleDogU291cmNlL0phdmFTY3JpcHRDb3JlL2xsaW50L0xvd0xldmVs SW50ZXJwcmV0ZXI2NC5hc20KPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL0phdmFTY3JpcHRDb3JlL2xs aW50L0xvd0xldmVsSW50ZXJwcmV0ZXI2NC5hc20JKHJldmlzaW9uIDE2MDc4NSkKKysrIFNvdXJj ZS9KYXZhU2NyaXB0Q29yZS9sbGludC9Mb3dMZXZlbEludGVycHJldGVyNjQuYXNtCSh3b3JraW5n IGNvcHkpCkBAIC0zMDAsNyArMzAwLDYgQEAgZW5kCiAKIG1hY3JvIHJlc3RvcmVTdGF0ZUFmdGVy Q0NhbGwoKQogICAgIG1vdmUgdDAsIFBDCi0gICAgbW92ZSB0MSwgY2ZyCiAgICAgbW92ZSB0Mywg UEIKICAgICBzdWJwIFBCLCBQQwogICAgIHJzaGlmdHAgMywgUEMKQEAgLTMzNiw3ICszMzUsNiBA QCBtYWNybyBjYWxsV2F0Y2hkb2dUaW1lckhhbmRsZXIodGhyb3dIYW5kCiAgICAgc3RvcmVpIFBD LCBBcmd1bWVudENvdW50ICsgVGFnT2Zmc2V0W2Nmcl0KICAgICBwcmVwYXJlU3RhdGVGb3JDQ2Fs bCgpCiAgICAgY0NhbGwyKF9sbGludF9zbG93X3BhdGhfaGFuZGxlX3dhdGNoZG9nX3RpbWVyLCBj ZnIsIFBDKQotICAgIG1vdmUgdDEsIGNmcgogICAgIGJ0cG56IHQwLCB0aHJvd0hhbmRsZXIKICAg ICBtb3ZlIHQzLCBQQgogICAgIGxvYWRpIEFyZ3VtZW50Q291bnQgKyBUYWdPZmZzZXRbY2ZyXSwg UEMKSW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9sbGludC9Mb3dMZXZlbEludGVycHJldGVy LmFzbQo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvSmF2YVNjcmlwdENvcmUvbGxpbnQvTG93TGV2ZWxJ bnRlcnByZXRlci5hc20JKHJldmlzaW9uIDE2MDc4NSkKKysrIFNvdXJjZS9KYXZhU2NyaXB0Q29y ZS9sbGludC9Mb3dMZXZlbEludGVycHJldGVyLmFzbQkod29ya2luZyBjb3B5KQpAQCAtMjkwLDcg KzI5MCw5IEBAIG1hY3JvIHNsb3dQYXRoRm9yQ2FsbChzbG93UGF0aCkKICAgICAgICAgICAgIGlm IENfTE9PUAogICAgICAgICAgICAgICAgIGNsb29wQ2FsbEpTRnVuY3Rpb24gY2FsbGVlCiAgICAg ICAgICAgICBlbHNlCisgICAgICAgICAgICAgICAgYnRweiB0MSwgLmRvbnRVcGRhdGVTUAogICAg ICAgICAgICAgICAgIGFkZHAgQ2FsbGVyRnJhbWVBbmRQQ1NpemUsIHQxLCBzcAorICAgICAgICAg ICAgLmRvbnRVcGRhdGVTUDoKICAgICAgICAgICAgICAgICBjYWxsIGNhbGxlZQogICAgICAgICAg ICAgICAgIHJlc3RvcmVTdGFja1BvaW50ZXJBZnRlckNhbGwoKQogICAgICAgICAgICAgICAgIGRp c3BhdGNoQWZ0ZXJDYWxsKCkKQEAgLTc5OCwxMCArODAwLDggQEAgX2xsaW50X29wX2NhbGxfdmFy YXJnczoKICAgICB0cmFjZUV4ZWN1dGlvbigpCiAgICAgY2FsbFNsb3dQYXRoKF9sbGludF9zbG93 X3BhdGhfc2l6ZV9hbmRfYWxsb2NfZnJhbWVfZm9yX3ZhcmFyZ3MpCiAgICAgYnJhbmNoSWZFeGNl cHRpb24oX2xsaW50X3Rocm93X2Zyb21fc2xvd19wYXRoX3RyYW1wb2xpbmUpCi0gICAgbG9hZHAg Q29kZUJsb2NrW2Nmcl0sIHQwCi0gICAgbG9hZHAgQ29kZUJsb2NrOjptX3ZtW3QwXSwgdDAKLSAg ICBsb2FkcCBWTTo6bmV3Q2FsbEZyYW1lUmV0dXJuVmFsdWVbdDBdLCB0MAotICAgIG1vdmUgdDAs IHNwCisgICAgIyBjYWxsZWVGcmFtZSBpbiB0MQorICAgIG1vdmUgdDEsIHNwCiAgICAgc2xvd1Bh dGhGb3JDYWxsKF9sbGludF9zbG93X3BhdGhfY2FsbF92YXJhcmdzKQogCiAK