67553 2011-09-02 22:06:11 -0700 ThunkGenerators does not convert positive double zero into integer zero 2011-09-03 22:42:58 -0700 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 1 fpizlo webkit-unassigned barraclough webkit.review.bot oldest_to_newest 461733 0 fpizlo 2011-09-02 22:06:11 -0700 ThunkGenerators does not convert positive double zero into integer zero, particularly when returning from floor, ceil, and round. This means that doing Math.floor(2/1) does not return an integer as some benchmarks expect. This omission is due to conservatism when dealing with double zero. A double zero could be negative, in which case converting to an integer would be wrong. But because the ThunkGenerators return the result in a GPR after boxing, it's possible to easily detect positive double zero. The first step of boxing is moving the double into a GPR. When the value is in a GPR, a positive double zero is represented by all bits being identically 0. Thus, a branchTestPtr(Zero, regT0) is all that is needed to detect positive double zero and convert it to an integer zero. 461735 1 106246 fpizlo 2011-09-02 22:11:17 -0700 Created attachment 106246 the patch 461747 2 106246 fpizlo 2011-09-02 22:27:28 -0700 Comment on attachment 106246 the patch All tests pass. 461802 3 barraclough 2011-09-03 01:43:29 -0700 Hey Filip, I haven't looked closely yet, but we really should make this work for JSVALUE32_64 too, if it doesn't do so already. 461803 4 fpizlo 2011-09-03 01:46:40 -0700 (In reply to comment #3) > Hey Filip, I haven't looked closely yet, but we really should make this work for JSVALUE32_64 too, if it doesn't do so already. It's not clear to me that this will be performance-neutral in JSVALUE32_64. On 64-bit it's a single branch. On JSVALUE32_64 it would be two branches. I'll mess around with it though. 461805 5 106258 fpizlo 2011-09-03 02:13:40 -0700 Created attachment 106258 the patch - 32 bit as well 461860 6 106258 barraclough 2011-09-03 14:57:16 -0700 Comment on attachment 106258 the patch - 32 bit as well Awesome, cheers for covering 32_64 too Phil. 461919 7 106258 webkit.review.bot 2011-09-03 22:42:54 -0700 Comment on attachment 106258 the patch - 32 bit as well Clearing flags on attachment: 106258 Committed r94500: <http://trac.webkit.org/changeset/94500> 461920 8 webkit.review.bot 2011-09-03 22:42:58 -0700 All reviewed patches have been landed. Closing bug. 106246 2011-09-02 22:11:17 -0700 2011-09-03 02:13:40 -0700 the patch doublezero_patch_1.diff text/plain 1575 fpizlo SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gOTQ0NzYpCisrKyBTb3VyY2Uv SmF2YVNjcmlwdENvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTUgQEAK KzIwMTEtMDktMDIgIEZpbGlwIFBpemxvICA8ZnBpemxvQGFwcGxlLmNvbT4KKworICAgICAgICBU aHVua0dlbmVyYXRvcnMgZG9lcyBub3QgY29udmVydCBwb3NpdGl2ZSBkb3VibGUgemVybyBpbnRv IGludGVnZXIgemVybworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5j Z2k/aWQ9Njc1NTMKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKyAgICAg ICAgCisgICAgICAgIFRoaXMgaXMgYW4gMC41JSBzcGVlZC11cCBvbiBWOCBhbmQgbmV1dHJhbCBl bHNld2hlcmUuCisKKyAgICAgICAgKiBqaXQvU3BlY2lhbGl6ZWRUaHVua0pJVC5oOgorICAgICAg ICAoSlNDOjpTcGVjaWFsaXplZFRodW5rSklUOjpyZXR1cm5Eb3VibGUpOgorCiAyMDExLTA5LTAy ICBNaWNoYWVsIFNhYm9mZiAgPG1zYWJvZmZAYXBwbGUuY29tPgogCiAgICAgICAgIFJlcGxhY2Ug bG9jYWwgaW1wbGVtZW50YXRpb24gb2Ygc3RyaW5nIGVxdWFscygpIG1ldGhvZHMgd2l0aCBVU3Ry aW5nIHZlcnNpb25zCkluZGV4OiBTb3VyY2UvSmF2YVNjcmlwdENvcmUvaml0L1NwZWNpYWxpemVk VGh1bmtKSVQuaAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvSmF2YVNjcmlwdENvcmUvaml0L1NwZWNp YWxpemVkVGh1bmtKSVQuaAkocmV2aXNpb24gOTQ0NzYpCisrKyBTb3VyY2UvSmF2YVNjcmlwdENv cmUvaml0L1NwZWNpYWxpemVkVGh1bmtKSVQuaAkod29ya2luZyBjb3B5KQpAQCAtOTUsNyArOTUs MTIgQEAgbmFtZXNwYWNlIEpTQyB7CiAgICAgICAgIHsKICNpZiBVU0UoSlNWQUxVRTY0KQogICAg ICAgICAgICAgbW92ZURvdWJsZVRvUHRyKHNyYywgcmVnVDApOworICAgICAgICAgICAgSnVtcCB6 ZXJvID0gYnJhbmNoVGVzdFB0cihaZXJvLCByZWdUMCk7CiAgICAgICAgICAgICBzdWJQdHIodGFn VHlwZU51bWJlclJlZ2lzdGVyLCByZWdUMCk7CisgICAgICAgICAgICBKdW1wIGRvbmUgPSBqdW1w KCk7CisgICAgICAgICAgICB6ZXJvLmxpbmsodGhpcyk7CisgICAgICAgICAgICBtb3ZlKHRhZ1R5 cGVOdW1iZXJSZWdpc3RlciwgcmVnVDApOworICAgICAgICAgICAgZG9uZS5saW5rKHRoaXMpOwog I2Vsc2UKICAgICAgICAgICAgIHN0b3JlRG91YmxlKHNyYywgQWRkcmVzcyhzdGFja1BvaW50ZXJS ZWdpc3RlciwgLShpbnQpc2l6ZW9mKGRvdWJsZSkpKTsKICAgICAgICAgICAgIGxvYWRQdHIoQWRk cmVzcyhzdGFja1BvaW50ZXJSZWdpc3RlciwgT0JKRUNUX09GRlNFVE9GKEpTVmFsdWUsIHUuYXNC aXRzLnRhZykgLSBzaXplb2YoZG91YmxlKSksIHJlZ1QxKTsK 106258 2011-09-03 02:13:40 -0700 2011-09-03 22:42:54 -0700 the patch - 32 bit as well doublezero_patch_2.diff text/plain 2115 fpizlo SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gOTQ0NzYpCisrKyBTb3VyY2Uv SmF2YVNjcmlwdENvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTUgQEAK KzIwMTEtMDktMDIgIEZpbGlwIFBpemxvICA8ZnBpemxvQGFwcGxlLmNvbT4KKworICAgICAgICBU aHVua0dlbmVyYXRvcnMgZG9lcyBub3QgY29udmVydCBwb3NpdGl2ZSBkb3VibGUgemVybyBpbnRv IGludGVnZXIgemVybworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5j Z2k/aWQ9Njc1NTMKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKyAgICAg ICAgCisgICAgICAgIFRoaXMgaXMgYW4gMC41JSBzcGVlZC11cCBvbiBWOCBhbmQgbmV1dHJhbCBl bHNld2hlcmUuCisKKyAgICAgICAgKiBqaXQvU3BlY2lhbGl6ZWRUaHVua0pJVC5oOgorICAgICAg ICAoSlNDOjpTcGVjaWFsaXplZFRodW5rSklUOjpyZXR1cm5Eb3VibGUpOgorCiAyMDExLTA5LTAy ICBNaWNoYWVsIFNhYm9mZiAgPG1zYWJvZmZAYXBwbGUuY29tPgogCiAgICAgICAgIFJlcGxhY2Ug bG9jYWwgaW1wbGVtZW50YXRpb24gb2Ygc3RyaW5nIGVxdWFscygpIG1ldGhvZHMgd2l0aCBVU3Ry aW5nIHZlcnNpb25zCkluZGV4OiBTb3VyY2UvSmF2YVNjcmlwdENvcmUvaml0L1NwZWNpYWxpemVk VGh1bmtKSVQuaAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvSmF2YVNjcmlwdENvcmUvaml0L1NwZWNp YWxpemVkVGh1bmtKSVQuaAkocmV2aXNpb24gOTQ0NzYpCisrKyBTb3VyY2UvSmF2YVNjcmlwdENv cmUvaml0L1NwZWNpYWxpemVkVGh1bmtKSVQuaAkod29ya2luZyBjb3B5KQpAQCAtOTUsMTEgKzk1 LDIyIEBAIG5hbWVzcGFjZSBKU0MgewogICAgICAgICB7CiAjaWYgVVNFKEpTVkFMVUU2NCkKICAg ICAgICAgICAgIG1vdmVEb3VibGVUb1B0cihzcmMsIHJlZ1QwKTsKKyAgICAgICAgICAgIEp1bXAg emVybyA9IGJyYW5jaFRlc3RQdHIoWmVybywgcmVnVDApOwogICAgICAgICAgICAgc3ViUHRyKHRh Z1R5cGVOdW1iZXJSZWdpc3RlciwgcmVnVDApOworICAgICAgICAgICAgSnVtcCBkb25lID0ganVt cCgpOworICAgICAgICAgICAgemVyby5saW5rKHRoaXMpOworICAgICAgICAgICAgbW92ZSh0YWdU eXBlTnVtYmVyUmVnaXN0ZXIsIHJlZ1QwKTsKKyAgICAgICAgICAgIGRvbmUubGluayh0aGlzKTsK ICNlbHNlCiAgICAgICAgICAgICBzdG9yZURvdWJsZShzcmMsIEFkZHJlc3Moc3RhY2tQb2ludGVy UmVnaXN0ZXIsIC0oaW50KXNpemVvZihkb3VibGUpKSk7CiAgICAgICAgICAgICBsb2FkUHRyKEFk ZHJlc3Moc3RhY2tQb2ludGVyUmVnaXN0ZXIsIE9CSkVDVF9PRkZTRVRPRihKU1ZhbHVlLCB1LmFz Qml0cy50YWcpIC0gc2l6ZW9mKGRvdWJsZSkpLCByZWdUMSk7CiAgICAgICAgICAgICBsb2FkUHRy KEFkZHJlc3Moc3RhY2tQb2ludGVyUmVnaXN0ZXIsIE9CSkVDVF9PRkZTRVRPRihKU1ZhbHVlLCB1 LmFzQml0cy5wYXlsb2FkKSAtIHNpemVvZihkb3VibGUpKSwgcmVnVDApOworICAgICAgICAgICAg SnVtcCBsb3dOb25aZXJvID0gYnJhbmNoVGVzdFB0cihOb25aZXJvLCByZWdUMSk7CisgICAgICAg ICAgICBKdW1wIGhpZ2hOb25aZXJvID0gYnJhbmNoVGVzdFB0cihOb25aZXJvLCByZWdUMCk7Cisg ICAgICAgICAgICBtb3ZlKFRydXN0ZWRJbW0zMigwKSwgcmVnVDApOworICAgICAgICAgICAgbW92 ZShUcnVzdGVkSW1tMzIoSW50MzJUYWcpLCByZWdUMSk7CisgICAgICAgICAgICBsb3dOb25aZXJv LmxpbmsodGhpcyk7CisgICAgICAgICAgICBoaWdoTm9uWmVyby5saW5rKHRoaXMpOwogI2VuZGlm CiAgICAgICAgICAgICBsb2FkUHRyKHBheWxvYWRGb3IoUmVnaXN0ZXJGaWxlOjpDYWxsZXJGcmFt ZSwgY2FsbEZyYW1lUmVnaXN0ZXIpLCBjYWxsRnJhbWVSZWdpc3Rlcik7CiAgICAgICAgICAgICBy ZXQoKTsK