152337 2015-12-16 08:52:14 -0800 Add "explicit operator bool" to ScratchRegisterAllocator::PreservedState 2015-12-16 10:37:09 -0800 1 1 1 Unclassified WebKit JavaScriptCore WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 saam saam commit-queue keith_miller mark.lam msaboff oldest_to_newest 1149613 0 saam 2015-12-16 08:52:14 -0800 We should assert we only get valid PreservedStates in restore reused registers. 1149619 1 267460 saam 2015-12-16 09:15:23 -0800 Created attachment 267460 patch 1149623 2 267460 mark.lam 2015-12-16 09:17:51 -0800 Comment on attachment 267460 patch r=me 1149624 3 267460 darin 2015-12-16 09:18:31 -0800 Comment on attachment 267460 patch View in context: https://bugs.webkit.org/attachment.cgi?id=267460&action=review > Source/JavaScriptCore/jit/ScratchRegisterAllocator.cpp:130 > + RELEASE_ASSERT(!!preservedState); Really surprised that we need !! here. I think that means there is something wrong with the RELEASE_ASSERT macro. > Source/JavaScriptCore/jit/ScratchRegisterAllocator.h:82 > + explicit operator bool() const { return numberOfBytesPreserved != std::numeric_limits<unsigned>::max(); } I am interested to learn that this is sufficient by itself without also defining operator! for the class. I think this means we can remove many of our operator! definitions throughout the WebKit project. 1149638 4 267460 saam 2015-12-16 09:38:19 -0800 Comment on attachment 267460 patch View in context: https://bugs.webkit.org/attachment.cgi?id=267460&action=review >> Source/JavaScriptCore/jit/ScratchRegisterAllocator.cpp:130 >> + RELEASE_ASSERT(!!preservedState); > > Really surprised that we need !! here. I think that means there is something wrong with the RELEASE_ASSERT macro. We don't. I just do this for stylistic reasons (or lack thereof). I happen to like the explicitness of how !! looks when testing the truthiness of something. This is probably overkill inside an assert. >> Source/JavaScriptCore/jit/ScratchRegisterAllocator.h:82 >> + explicit operator bool() const { return numberOfBytesPreserved != std::numeric_limits<unsigned>::max(); } > > I am interested to learn that this is sufficient by itself without also defining operator! for the class. I think this means we can remove many of our operator! definitions throughout the WebKit project. I think one of the main benefits of "explicit operator bool" is that it eliminates most of the need for operator!. 1149677 5 saam 2015-12-16 10:37:09 -0800 landed in: http://trac.webkit.org/changeset/194158 267460 2015-12-16 09:15:23 -0800 2015-12-16 09:17:51 -0800 patch b-backup.diff text/plain 3282 saam SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMTk0MTQzKQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDIwIEBA CisyMDE1LTEyLTE2ICBTYWFtIGJhcmF0aSAgPHNiYXJhdGlAYXBwbGUuY29tPgorCisgICAgICAg IEFkZCAiZXhwbGljaXQgb3BlcmF0b3IgYm9vbCIgdG8gU2NyYXRjaFJlZ2lzdGVyQWxsb2NhdG9y OjpQcmVzZXJ2ZWRTdGF0ZQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1 Zy5jZ2k/aWQ9MTUyMzM3CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisK KyAgICAgICAgSWYgd2UgaGF2ZSBhIGRlZmF1bHQgY29uc3RydWN0b3IsIHdlIHNob3VsZCBhbHNv IGhhdmUgYSB3YXkKKyAgICAgICAgdG8gdGVsbCBpZiBhIFByZXNlcnZlZFN0YXRlIGlzIGludmFs aWQuCisKKyAgICAgICAgKiBqaXQvU2NyYXRjaFJlZ2lzdGVyQWxsb2NhdG9yLmNwcDoKKyAgICAg ICAgKEpTQzo6U2NyYXRjaFJlZ2lzdGVyQWxsb2NhdG9yOjpwcmVzZXJ2ZVJldXNlZFJlZ2lzdGVy c0J5UHVzaGluZyk6CisgICAgICAgIChKU0M6OlNjcmF0Y2hSZWdpc3RlckFsbG9jYXRvcjo6cmVz dG9yZVJldXNlZFJlZ2lzdGVyc0J5UG9wcGluZyk6CisgICAgICAgICogaml0L1NjcmF0Y2hSZWdp c3RlckFsbG9jYXRvci5oOgorICAgICAgICAoSlNDOjpTY3JhdGNoUmVnaXN0ZXJBbGxvY2F0b3I6 OlByZXNlcnZlZFN0YXRlOjpQcmVzZXJ2ZWRTdGF0ZSk6CisgICAgICAgIChKU0M6OlNjcmF0Y2hS ZWdpc3RlckFsbG9jYXRvcjo6UHJlc2VydmVkU3RhdGU6Om9wZXJhdG9yIGJvb2wpOgorCiAyMDE1 LTEyLTE2ICBDb21taXQgUXVldWUgIDxjb21taXQtcXVldWVAd2Via2l0Lm9yZz4KIAogICAgICAg ICBVbnJldmlld2VkLCByb2xsaW5nIG91dCByMTk0MTM1LgpJbmRleDogU291cmNlL0phdmFTY3Jp cHRDb3JlL2ppdC9TY3JhdGNoUmVnaXN0ZXJBbGxvY2F0b3IuY3BwCj09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNv dXJjZS9KYXZhU2NyaXB0Q29yZS9qaXQvU2NyYXRjaFJlZ2lzdGVyQWxsb2NhdG9yLmNwcAkocmV2 aXNpb24gMTk0MTQzKQorKysgU291cmNlL0phdmFTY3JpcHRDb3JlL2ppdC9TY3JhdGNoUmVnaXN0 ZXJBbGxvY2F0b3IuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC0xMjUsOCArMTI1LDkgQEAgU2NyYXRj aFJlZ2lzdGVyQWxsb2NhdG9yOjpQcmVzZXJ2ZWRTdGF0ZQogICAgIHJldHVybiBQcmVzZXJ2ZWRT dGF0ZShzdGFja0FkanVzdG1lbnRTaXplLCBleHRyYVN0YWNrU3BhY2UpOwogfQogCi12b2lkIFNj cmF0Y2hSZWdpc3RlckFsbG9jYXRvcjo6cmVzdG9yZVJldXNlZFJlZ2lzdGVyc0J5UG9wcGluZyhN YWNyb0Fzc2VtYmxlciYgaml0LCBjb25zdCBTY3JhdGNoUmVnaXN0ZXJBbGxvY2F0b3I6OlByZXNl cnZlZFN0YXRlIHByZXNlcnZlZFN0YXRlKQordm9pZCBTY3JhdGNoUmVnaXN0ZXJBbGxvY2F0b3I6 OnJlc3RvcmVSZXVzZWRSZWdpc3RlcnNCeVBvcHBpbmcoTWFjcm9Bc3NlbWJsZXImIGppdCwgY29u c3QgU2NyYXRjaFJlZ2lzdGVyQWxsb2NhdG9yOjpQcmVzZXJ2ZWRTdGF0ZSYgcHJlc2VydmVkU3Rh dGUpCiB7CisgICAgUkVMRUFTRV9BU1NFUlQoISFwcmVzZXJ2ZWRTdGF0ZSk7CiAgICAgaWYgKCFk aWRSZXVzZVJlZ2lzdGVycygpKQogICAgICAgICByZXR1cm47CiAKSW5kZXg6IFNvdXJjZS9KYXZh U2NyaXB0Q29yZS9qaXQvU2NyYXRjaFJlZ2lzdGVyQWxsb2NhdG9yLmgKPT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0g U291cmNlL0phdmFTY3JpcHRDb3JlL2ppdC9TY3JhdGNoUmVnaXN0ZXJBbGxvY2F0b3IuaAkocmV2 aXNpb24gMTk0MTQzKQorKysgU291cmNlL0phdmFTY3JpcHRDb3JlL2ppdC9TY3JhdGNoUmVnaXN0 ZXJBbGxvY2F0b3IuaAkod29ya2luZyBjb3B5KQpAQCAtNzAsMjAgKzcwLDIzIEBAIHB1YmxpYzoK IAogICAgIHN0cnVjdCBQcmVzZXJ2ZWRTdGF0ZSB7CiAgICAgICAgIFByZXNlcnZlZFN0YXRlKCkK LSAgICAgICAgICAgIDogUHJlc2VydmVkU3RhdGUoMCkKKyAgICAgICAgICAgIDogbnVtYmVyT2ZC eXRlc1ByZXNlcnZlZChzdGQ6Om51bWVyaWNfbGltaXRzPHVuc2lnbmVkPjo6bWF4KCkpCisgICAg ICAgICAgICAsIGV4dHJhU3RhY2tTcGFjZVJlcXVpcmVtZW50KEV4dHJhU3RhY2tTcGFjZTo6U3Bh Y2VGb3JDQ2FsbCkKICAgICAgICAgeyB9CiAKLSAgICAgICAgUHJlc2VydmVkU3RhdGUodW5zaWdu ZWQgbnVtYmVyT2ZCeXRlcywgRXh0cmFTdGFja1NwYWNlIGV4dHJhU3RhY2tTcGFjZSA9IEV4dHJh U3RhY2tTcGFjZTo6Tm9FeHRyYVNwYWNlKQorICAgICAgICBQcmVzZXJ2ZWRTdGF0ZSh1bnNpZ25l ZCBudW1iZXJPZkJ5dGVzLCBFeHRyYVN0YWNrU3BhY2UgZXh0cmFTdGFja1NwYWNlKQogICAgICAg ICAgICAgOiBudW1iZXJPZkJ5dGVzUHJlc2VydmVkKG51bWJlck9mQnl0ZXMpCiAgICAgICAgICAg ICAsIGV4dHJhU3RhY2tTcGFjZVJlcXVpcmVtZW50KGV4dHJhU3RhY2tTcGFjZSkKICAgICAgICAg eyB9CiAKKyAgICAgICAgZXhwbGljaXQgb3BlcmF0b3IgYm9vbCgpIGNvbnN0IHsgcmV0dXJuIG51 bWJlck9mQnl0ZXNQcmVzZXJ2ZWQgIT0gc3RkOjpudW1lcmljX2xpbWl0czx1bnNpZ25lZD46Om1h eCgpOyB9CisKICAgICAgICAgdW5zaWduZWQgbnVtYmVyT2ZCeXRlc1ByZXNlcnZlZDsKICAgICAg ICAgRXh0cmFTdGFja1NwYWNlIGV4dHJhU3RhY2tTcGFjZVJlcXVpcmVtZW50OwogICAgIH07CiAK ICAgICBQcmVzZXJ2ZWRTdGF0ZSBwcmVzZXJ2ZVJldXNlZFJlZ2lzdGVyc0J5UHVzaGluZyhNYWNy b0Fzc2VtYmxlciYgaml0LCBFeHRyYVN0YWNrU3BhY2UpOwotICAgIHZvaWQgcmVzdG9yZVJldXNl ZFJlZ2lzdGVyc0J5UG9wcGluZyhNYWNyb0Fzc2VtYmxlciYgaml0LCBQcmVzZXJ2ZWRTdGF0ZSk7 CisgICAgdm9pZCByZXN0b3JlUmV1c2VkUmVnaXN0ZXJzQnlQb3BwaW5nKE1hY3JvQXNzZW1ibGVy JiBqaXQsIGNvbnN0IFByZXNlcnZlZFN0YXRlJik7CiAgICAgCiAgICAgUmVnaXN0ZXJTZXQgdXNl ZFJlZ2lzdGVyc0ZvckNhbGwoKSBjb25zdDsKICAgICAK