183416 2018-03-07 14:30:32 -0800 Make it possible to randomize register allocation 2018-03-12 10:08:49 -0700 1 1 1 Unclassified WebKit JavaScriptCore WebKit Nightly Build All All RESOLVED FIXED InRadar P2 Normal --- 1 fpizlo fpizlo benjamin cdumez cmarcelo darin dbates ews-watchlist keith_miller mark.lam msaboff saam webkit-bug-importer oldest_to_newest 1404478 0 fpizlo 2018-03-07 14:30:32 -0800 Patch forthcoming. 1404480 1 335224 fpizlo 2018-03-07 14:31:36 -0800 Created attachment 335224 the patch 1404481 2 ews-watchlist 2018-03-07 14:36:12 -0800 Attachment 335224 did not pass style-queue: ERROR: Source/JavaScriptCore/b3/air/AirCode.cpp:79: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/b3/air/AirCode.cpp:80: More than one command on the same line [whitespace/newline] [4] Total errors found: 2 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style. 1404485 3 335224 keith_miller 2018-03-07 14:41:30 -0800 Comment on attachment 335224 the patch View in context: https://bugs.webkit.org/attachment.cgi?id=335224&action=review > Source/JavaScriptCore/b3/air/AirCode.cpp:62 > + Vector<Reg> volatileRegs; Why not call this callerSaveRegs? Is it to avoid typo bugs? > Source/JavaScriptCore/b3/air/AirCode.cpp:65 > all.exclude(RegisterSet::stackRegisters()); I think the wasm bug might be that you are not excluding the pinned registers? 1404486 4 335224 keith_miller 2018-03-07 14:43:09 -0800 Comment on attachment 335224 the patch View in context: https://bugs.webkit.org/attachment.cgi?id=335224&action=review >> Source/JavaScriptCore/b3/air/AirCode.cpp:65 >> all.exclude(RegisterSet::stackRegisters()); > > I think the wasm bug might be that you are not excluding the pinned registers? Nvm, I don't think this is the issue. Ignore me. 1404488 5 keith_miller 2018-03-07 14:43:37 -0800 r=me. 1404744 6 fpizlo 2018-03-08 09:03:07 -0800 (In reply to Keith Miller from comment #3) > Comment on attachment 335224 [details] > the patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=335224&action=review > > > Source/JavaScriptCore/b3/air/AirCode.cpp:62 > > + Vector<Reg> volatileRegs; > > Why not call this callerSaveRegs? Is it to avoid typo bugs? Yeah. I don't really know what the best terminology is, but I have often used "volatile" versus "callee-save", because it's easier to distinguish the two. > > > Source/JavaScriptCore/b3/air/AirCode.cpp:65 > > all.exclude(RegisterSet::stackRegisters()); > > I think the wasm bug might be that you are not excluding the pinned > registers? Nah. This is just about order. The pinned regs will get excluded anyway. 1404754 7 fpizlo 2018-03-08 09:12:16 -0800 Landed in https://trac.webkit.org/changeset/229412/webkit 1404773 8 webkit-bug-importer 2018-03-08 09:30:59 -0800 <rdar://problem/38266059> 1405518 9 335224 darin 2018-03-10 17:40:39 -0800 Comment on attachment 335224 the patch View in context: https://bugs.webkit.org/attachment.cgi?id=335224&action=review > Source/WTF/wtf/MathExtras.h:534 > +template<typename VectorType, typename RandomFunc> > +void shuffleVector(VectorType& vector, size_t size, const RandomFunc& randomFunc) > +{ > + for (size_t i = 0; i + 1 < size; ++i) > + std::swap(vector[i], vector[i + randomFunc(size - i)]); > +} Why doesn’t this use std::random_shuffle? 1405774 10 keith_miller 2018-03-12 10:08:49 -0700 (In reply to Darin Adler from comment #9) > Comment on attachment 335224 [details] > the patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=335224&action=review > > > Source/WTF/wtf/MathExtras.h:534 > > +template<typename VectorType, typename RandomFunc> > > +void shuffleVector(VectorType& vector, size_t size, const RandomFunc& randomFunc) > > +{ > > + for (size_t i = 0; i + 1 < size; ++i) > > + std::swap(vector[i], vector[i + randomFunc(size - i)]); > > +} > > Why doesn’t this use std::random_shuffle? According to http://en.cppreference.com/w/cpp/algorithm/random_shuffle, std::random_shuffle is deprecated in C++14 and removed in C++17 so it probably doesn't make sense to use it. We could use std::shuffle but that has a really obnoxious API... 335224 2018-03-07 14:31:36 -0800 2018-03-07 14:43:29 -0800 the patch blah.patch text/plain 6596 fpizlo SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMjI5Mzc1KQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE4IEBA CisyMDE4LTAzLTA3ICBGaWxpcCBQaXpsbyAgPGZwaXpsb0BhcHBsZS5jb20+CisKKyAgICAgICAg TWFrZSBpdCBwb3NzaWJsZSB0byByYW5kb21pemUgcmVnaXN0ZXIgYWxsb2NhdGlvbgorICAgICAg ICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTgzNDE2CisKKyAgICAg ICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisgICAgICAgIAorICAgICAgICBUaGlzIGlz IGRpc2FibGVkIGJ5IGRlZmF1bHQgZm9yIG5vdywgYmVjYXVzZSBpdCByZXZlYWxzIGEgcmVnYWxs b2MgYnVnIGluIHdhc20uCisKKyAgICAgICAgKiBiMy9haXIvQWlyQ29kZS5jcHA6CisgICAgICAg IChKU0M6OkIzOjpBaXI6OkNvZGU6OkNvZGUpOgorICAgICAgICAqIGIzL2Fpci9BaXJDb2RlLmg6 CisgICAgICAgIChKU0M6OkIzOjpBaXI6OkNvZGU6OndlYWtSYW5kb20pOgorICAgICAgICAqIHJ1 bnRpbWUvT3B0aW9ucy5oOgorCiAyMDE4LTAzLTA3ICBNYXJrIExhbSAgPG1hcmsubGFtQGFwcGxl LmNvbT4KIAogICAgICAgICBSZW5hbWUgUHJvdG9DYWxsRnJhbWU6OmFyaXR5TWlzc01hdGNoIHRv IGhhc0FyaXR5TWlzbWF0Y2guCkluZGV4OiBTb3VyY2UvSmF2YVNjcmlwdENvcmUvYjMvYWlyL0Fp ckNvZGUuY3BwCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9iMy9haXIvQWly Q29kZS5jcHAJKHJldmlzaW9uIDIyODcxNSkKKysrIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9iMy9h aXIvQWlyQ29kZS5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTEsNSArMSw1IEBACiAvKgotICogQ29w eXJpZ2h0IChDKSAyMDE1LTIwMTcgQXBwbGUgSW5jLiBBbGwgcmlnaHRzIHJlc2VydmVkLgorICog Q29weXJpZ2h0IChDKSAyMDE1LTIwMTggQXBwbGUgSW5jLiBBbGwgcmlnaHRzIHJlc2VydmVkLgog ICoKICAqIFJlZGlzdHJpYnV0aW9uIGFuZCB1c2UgaW4gc291cmNlIGFuZCBiaW5hcnkgZm9ybXMs IHdpdGggb3Igd2l0aG91dAogICogbW9kaWZpY2F0aW9uLCBhcmUgcGVybWl0dGVkIHByb3ZpZGVk IHRoYXQgdGhlIGZvbGxvd2luZyBjb25kaXRpb25zCkBAIC0zNSw2ICszNSw3IEBACiAjaW5jbHVk ZSAiQjNQcm9jZWR1cmUuaCIKICNpbmNsdWRlICJCM1N0YWNrU2xvdC5oIgogI2luY2x1ZGUgPHd0 Zi9MaXN0RHVtcC5oPgorI2luY2x1ZGUgPHd0Zi9NYXRoRXh0cmFzLmg+CiAKIG5hbWVzcGFjZSBK U0MgeyBuYW1lc3BhY2UgQjMgeyBuYW1lc3BhY2UgQWlyIHsKIApAQCAtNTgsNyArNTksOCBAQCBD b2RlOjpDb2RlKFByb2NlZHVyZSYgcHJvYykKICAgICAvLyBDb21lIHVwIHdpdGggaW5pdGlhbCBv cmRlcmluZ3Mgb2YgcmVnaXN0ZXJzLiBUaGUgdXNlciBtYXkgcmVwbGFjZSB0aGlzIHdpdGggc29t ZXRoaW5nIGVsc2UuCiAgICAgZm9yRWFjaEJhbmsoCiAgICAgICAgIFsmXSAoQmFuayBiYW5rKSB7 Ci0gICAgICAgICAgICBWZWN0b3I8UmVnPiByZXN1bHQ7CisgICAgICAgICAgICBWZWN0b3I8UmVn PiB2b2xhdGlsZVJlZ3M7CisgICAgICAgICAgICBWZWN0b3I8UmVnPiBjYWxsZWVTYXZlUmVnczsK ICAgICAgICAgICAgIFJlZ2lzdGVyU2V0IGFsbCA9IGJhbmsgPT0gR1AgPyBSZWdpc3RlclNldDo6 YWxsR1BScygpIDogUmVnaXN0ZXJTZXQ6OmFsbEZQUnMoKTsKICAgICAgICAgICAgIGFsbC5leGNs dWRlKFJlZ2lzdGVyU2V0OjpzdGFja1JlZ2lzdGVycygpKTsKICAgICAgICAgICAgIGFsbC5leGNs dWRlKFJlZ2lzdGVyU2V0OjpyZXNlcnZlZEhhcmR3YXJlUmVnaXN0ZXJzKCkpOwpAQCAtNjYsMTMg KzY4LDIwIEBAIENvZGU6OkNvZGUoUHJvY2VkdXJlJiBwcm9jKQogICAgICAgICAgICAgYWxsLmZv ckVhY2goCiAgICAgICAgICAgICAgICAgWyZdIChSZWcgcmVnKSB7CiAgICAgICAgICAgICAgICAg ICAgIGlmICghY2FsbGVlU2F2ZS5nZXQocmVnKSkKLSAgICAgICAgICAgICAgICAgICAgICAgIHJl c3VsdC5hcHBlbmQocmVnKTsKKyAgICAgICAgICAgICAgICAgICAgICAgIHZvbGF0aWxlUmVncy5h cHBlbmQocmVnKTsKICAgICAgICAgICAgICAgICB9KTsKICAgICAgICAgICAgIGFsbC5mb3JFYWNo KAogICAgICAgICAgICAgICAgIFsmXSAoUmVnIHJlZykgewogICAgICAgICAgICAgICAgICAgICBp ZiAoY2FsbGVlU2F2ZS5nZXQocmVnKSkKLSAgICAgICAgICAgICAgICAgICAgICAgIHJlc3VsdC5h cHBlbmQocmVnKTsKKyAgICAgICAgICAgICAgICAgICAgICAgIGNhbGxlZVNhdmVSZWdzLmFwcGVu ZChyZWcpOwogICAgICAgICAgICAgICAgIH0pOworICAgICAgICAgICAgaWYgKE9wdGlvbnM6OmFp clJhbmRvbWl6ZVJlZ3MoKSkgeworICAgICAgICAgICAgICAgIHNodWZmbGVWZWN0b3Iodm9sYXRp bGVSZWdzLCBbJl0gKHVuc2lnbmVkIGxpbWl0KSB7IHJldHVybiBtX3dlYWtSYW5kb20uZ2V0VWlu dDMyKGxpbWl0KTsgfSk7CisgICAgICAgICAgICAgICAgc2h1ZmZsZVZlY3RvcihjYWxsZWVTYXZl UmVncywgWyZdICh1bnNpZ25lZCBsaW1pdCkgeyByZXR1cm4gbV93ZWFrUmFuZG9tLmdldFVpbnQz MihsaW1pdCk7IH0pOworICAgICAgICAgICAgfQorICAgICAgICAgICAgVmVjdG9yPFJlZz4gcmVz dWx0OworICAgICAgICAgICAgcmVzdWx0LmFwcGVuZFZlY3Rvcih2b2xhdGlsZVJlZ3MpOworICAg ICAgICAgICAgcmVzdWx0LmFwcGVuZFZlY3RvcihjYWxsZWVTYXZlUmVncyk7CiAgICAgICAgICAg ICBzZXRSZWdzSW5Qcmlvcml0eU9yZGVyKGJhbmssIHJlc3VsdCk7CiAgICAgICAgIH0pOwogCklu ZGV4OiBTb3VyY2UvSmF2YVNjcmlwdENvcmUvYjMvYWlyL0FpckNvZGUuaAo9PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0t LSBTb3VyY2UvSmF2YVNjcmlwdENvcmUvYjMvYWlyL0FpckNvZGUuaAkocmV2aXNpb24gMjI4NzE1 KQorKysgU291cmNlL0phdmFTY3JpcHRDb3JlL2IzL2Fpci9BaXJDb2RlLmgJKHdvcmtpbmcgY29w eSkKQEAgLTEsNSArMSw1IEBACiAvKgotICogQ29weXJpZ2h0IChDKSAyMDE1LTIwMTcgQXBwbGUg SW5jLiBBbGwgcmlnaHRzIHJlc2VydmVkLgorICogQ29weXJpZ2h0IChDKSAyMDE1LTIwMTggQXBw bGUgSW5jLiBBbGwgcmlnaHRzIHJlc2VydmVkLgogICoKICAqIFJlZGlzdHJpYnV0aW9uIGFuZCB1 c2UgaW4gc291cmNlIGFuZCBiaW5hcnkgZm9ybXMsIHdpdGggb3Igd2l0aG91dAogICogbW9kaWZp Y2F0aW9uLCBhcmUgcGVybWl0dGVkIHByb3ZpZGVkIHRoYXQgdGhlIGZvbGxvd2luZyBjb25kaXRp b25zCkBAIC0zOCw2ICszOCw3IEBACiAjaW5jbHVkZSAiUmVnaXN0ZXJBdE9mZnNldExpc3QuaCIK ICNpbmNsdWRlICJTdGFja0FsaWdubWVudC5oIgogI2luY2x1ZGUgPHd0Zi9JbmRleE1hcC5oPgor I2luY2x1ZGUgPHd0Zi9XZWFrUmFuZG9tLmg+CiAKIG5hbWVzcGFjZSBKU0MgeyBuYW1lc3BhY2Ug QjMgewogCkBAIC0zMzMsNyArMzM0LDkgQEAgcHVibGljOgogICAgIFJlZ2lzdGVyU2V0IG11dGFi bGVHUFJzKCk7CiAgICAgUmVnaXN0ZXJTZXQgbXV0YWJsZUZQUnMoKTsKICAgICBSZWdpc3RlclNl dCBwaW5uZWRSZWdpc3RlcnMoKSBjb25zdCB7IHJldHVybiBtX3Bpbm5lZFJlZ3M7IH0KLQorICAg IAorICAgIFdlYWtSYW5kb20mIHdlYWtSYW5kb20oKSB7IHJldHVybiBtX3dlYWtSYW5kb207IH0K KyAgICAKIHByaXZhdGU6CiAgICAgZnJpZW5kIGNsYXNzIDo6SlNDOjpCMzo6UHJvY2VkdXJlOwog ICAgIGZyaWVuZCBjbGFzcyBCbG9ja0luc2VydGlvblNldDsKQEAgLTM1Myw2ICszNTYsNyBAQCBw cml2YXRlOgogICAgICAgICBBU1NFUlRfTk9UX1JFQUNIRUQoKTsKICAgICB9CiAKKyAgICBXZWFr UmFuZG9tIG1fd2Vha1JhbmRvbTsKICAgICBQcm9jZWR1cmUmIG1fcHJvYzsgLy8gU29tZSBtZXRh LWRhdGEsIGxpa2UgYnlwcm9kdWN0cywgaXMgc3RvcmVkIGluIHRoZSBQcm9jZWR1cmUuCiAgICAg VmVjdG9yPFJlZz4gbV9ncFJlZ3NJblByaW9yaXR5T3JkZXI7CiAgICAgVmVjdG9yPFJlZz4gbV9m cFJlZ3NJblByaW9yaXR5T3JkZXI7CkluZGV4OiBTb3VyY2UvSmF2YVNjcmlwdENvcmUvcnVudGlt ZS9PcHRpb25zLmgKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL0phdmFTY3JpcHRDb3JlL3J1bnRpbWUv T3B0aW9ucy5oCShyZXZpc2lvbiAyMjg3MTUpCisrKyBTb3VyY2UvSmF2YVNjcmlwdENvcmUvcnVu dGltZS9PcHRpb25zLmgJKHdvcmtpbmcgY29weSkKQEAgLTQyNiw2ICs0MjYsNyBAQCBjb25zdGV4 cHIgYm9vbCBlbmFibGVBc3luY0l0ZXJhdGlvbiA9IGZhCiAgICAgdihib29sLCBhaXJMaW5lYXJT Y2FuU3BpbGxzRXZlcnl0aGluZywgZmFsc2UsIE5vcm1hbCwgbnVsbHB0cikgXAogICAgIHYoYm9v bCwgYWlyRm9yY2VCcmlnZ3NBbGxvY2F0b3IsIGZhbHNlLCBOb3JtYWwsIG51bGxwdHIpIFwKICAg ICB2KGJvb2wsIGFpckZvcmNlSVJDQWxsb2NhdG9yLCBmYWxzZSwgTm9ybWFsLCBudWxscHRyKSBc CisgICAgdihib29sLCBhaXJSYW5kb21pemVSZWdzLCBmYWxzZSwgTm9ybWFsLCBudWxscHRyKSBc CiAgICAgdihib29sLCBjb2FsZXNjZVNwaWxsU2xvdHMsIHRydWUsIE5vcm1hbCwgbnVsbHB0cikg XAogICAgIHYoYm9vbCwgbG9nQWlyUmVnaXN0ZXJQcmVzc3VyZSwgZmFsc2UsIE5vcm1hbCwgbnVs bHB0cikgXAogICAgIHYodW5zaWduZWQsIG1heEIzVGFpbER1cEJsb2NrU2l6ZSwgMywgTm9ybWFs LCBudWxscHRyKSBcCkluZGV4OiBTb3VyY2UvV1RGL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBT b3VyY2UvV1RGL0NoYW5nZUxvZwkocmV2aXNpb24gMjI4NzE1KQorKysgU291cmNlL1dURi9DaGFu Z2VMb2cJKHdvcmtpbmcgY29weSkKQEAgLTEsMyArMSwxMyBAQAorMjAxOC0wMy0wNyAgRmlsaXAg UGl6bG8gIDxmcGl6bG9AYXBwbGUuY29tPgorCisgICAgICAgIE1ha2UgaXQgcG9zc2libGUgdG8g cmFuZG9taXplIHJlZ2lzdGVyIGFsbG9jYXRpb24KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtp dC5vcmcvc2hvd19idWcuY2dpP2lkPTE4MzQxNgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9E WSAoT09QUyEpLgorCisgICAgICAgICogd3RmL01hdGhFeHRyYXMuaDoKKyAgICAgICAgKFdURjo6 c2h1ZmZsZVZlY3Rvcik6CisKIDIwMTgtMDItMTYgIERlYW4gSmFja3NvbiAgPGRpbm9AYXBwbGUu Y29tPgogCiAgICAgICAgIFVzZSBPUEVOR0wgbWFjcm9zIHRvIGJlIG1vcmUgY2xlYXIgYWJvdXQg d2hpY2ggT3BlbkdML0VTIFdlYkdMIHVzZXMgb24gQ29jb2EKSW5kZXg6IFNvdXJjZS9XVEYvd3Rm L01hdGhFeHRyYXMuaAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV1RGL3d0Zi9NYXRoRXh0cmFzLmgJ KHJldmlzaW9uIDIyODcxNSkKKysrIFNvdXJjZS9XVEYvd3RmL01hdGhFeHRyYXMuaAkod29ya2lu ZyBjb3B5KQpAQCAtNTI2LDYgKzUyNiwxOSBAQCBpbmxpbmUgVCogZHluYW1pY1BvaXNvbihVIGFj dHVhbCwgVSBleHBlCiAjZW5kaWYKIH0KIAordGVtcGxhdGU8dHlwZW5hbWUgVmVjdG9yVHlwZSwg dHlwZW5hbWUgUmFuZG9tRnVuYz4KK3ZvaWQgc2h1ZmZsZVZlY3RvcihWZWN0b3JUeXBlJiB2ZWN0 b3IsIHNpemVfdCBzaXplLCBjb25zdCBSYW5kb21GdW5jJiByYW5kb21GdW5jKQoreworICAgIGZv ciAoc2l6ZV90IGkgPSAwOyBpICsgMSA8IHNpemU7ICsraSkKKyAgICAgICAgc3RkOjpzd2FwKHZl Y3RvcltpXSwgdmVjdG9yW2kgKyByYW5kb21GdW5jKHNpemUgLSBpKV0pOworfQorCit0ZW1wbGF0 ZTx0eXBlbmFtZSBWZWN0b3JUeXBlLCB0eXBlbmFtZSBSYW5kb21GdW5jPgordm9pZCBzaHVmZmxl VmVjdG9yKFZlY3RvclR5cGUmIHZlY3RvciwgY29uc3QgUmFuZG9tRnVuYyYgcmFuZG9tRnVuYykK K3sKKyAgICBzaHVmZmxlVmVjdG9yKHZlY3RvciwgdmVjdG9yLnNpemUoKSwgcmFuZG9tRnVuYyk7 Cit9CisKIH0gLy8gbmFtZXNwYWNlIFdURgogCiB1c2luZyBXVEY6OmR5bmFtaWNQb2lzb247CkBA IC01MzMsNSArNTQ2LDYgQEAgdXNpbmcgV1RGOjpvcGFxdWU7CiB1c2luZyBXVEY6OnByZWNpc2VJ bmRleE1hc2tQdHI7CiB1c2luZyBXVEY6OnByZWNpc2VJbmRleE1hc2tTaGlmdDsKIHVzaW5nIFdU Rjo6cHJlY2lzZUluZGV4TWFza1NoaWZ0Rm9yU2l6ZTsKK3VzaW5nIFdURjo6c2h1ZmZsZVZlY3Rv cjsKIAogI2VuZGlmIC8vICNpZm5kZWYgV1RGX01hdGhFeHRyYXNfaAo=