109900 2013-02-14 23:28:34 -0800 DFG AbstractState should filter operands to NewArray more precisely 2013-02-15 11:58:51 -0800 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 1 fpizlo fpizlo abarth barraclough ggaren mark.lam mhahnenberg msaboff oliver sam oldest_to_newest 833735 0 fpizlo 2013-02-14 23:28:34 -0800 Patch forthcoming 833736 1 188489 fpizlo 2013-02-14 23:32:21 -0800 Created attachment 188489 the patch 833999 2 188489 mhahnenberg 2013-02-15 07:20:12 -0800 Comment on attachment 188489 the patch View in context: https://bugs.webkit.org/attachment.cgi?id=188489&action=review > Source/JavaScriptCore/dfg/DFGAbstractState.cpp:1300 > + forNode(m_graph.m_varArgChildren[node->firstChild() + operandIndex]).filter(SpecRealNumber); Why not SpecDouble? What would happen if somebody filled their arrays with NaNs? 834218 3 fpizlo 2013-02-15 11:37:40 -0800 (In reply to comment #2) > (From update of attachment 188489 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=188489&action=review > > > Source/JavaScriptCore/dfg/DFGAbstractState.cpp:1300 > > + forNode(m_graph.m_varArgChildren[node->firstChild() + operandIndex]).filter(SpecRealNumber); > > Why not SpecDouble? What would happen if somebody filled their arrays with NaNs? NaNs can't be stored into double arrays. If you do it, they turn into contiguous arrays (of generic JSValues). The backend will speculate that you're not storing NaN into a double array and spec fail if you do (so that the baseline JIT can do the double->contiguous conversion). Hence, filtering SpecRealNumber accurately represents the speculations that the backend will do. 834220 4 188489 mhahnenberg 2013-02-15 11:38:25 -0800 Comment on attachment 188489 the patch View in context: https://bugs.webkit.org/attachment.cgi?id=188489&action=review r=me >>> Source/JavaScriptCore/dfg/DFGAbstractState.cpp:1300 >>> + forNode(m_graph.m_varArgChildren[node->firstChild() + operandIndex]).filter(SpecRealNumber); >> >> Why not SpecDouble? What would happen if somebody filled their arrays with NaNs? > > NaNs can't be stored into double arrays. If you do it, they turn into contiguous arrays (of generic JSValues). The backend will speculate that you're not storing NaN into a double array and spec fail if you do (so that the baseline JIT can do the double->contiguous conversion). > > Hence, filtering SpecRealNumber accurately represents the speculations that the backend will do. Sounds good. 834224 5 fpizlo 2013-02-15 11:49:39 -0800 Landed in http://trac.webkit.org/changeset/143024 834240 6 abarth 2013-02-15 11:57:43 -0800 Reopening to attach new patch. 834241 7 abarth 2013-02-15 11:58:51 -0800 oops. soryr 188489 2013-02-14 23:32:21 -0800 2013-02-15 11:57:41 -0800 the patch blah.patch text/plain 2362 fpizlo SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMTQyOTY4KQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE2IEBA CisyMDEzLTAyLTE0ICBGaWxpcCBQaXpsbyAgPGZwaXpsb0BhcHBsZS5jb20+CisKKyAgICAgICAg REZHIEFic3RyYWN0U3RhdGUgc2hvdWxkIGZpbHRlciBvcGVyYW5kcyB0byBOZXdBcnJheSBtb3Jl IHByZWNpc2VseQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/ aWQ9MTA5OTAwCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisgICAgICAg IAorICAgICAgICBOZXdBcnJheSBmb3IgcHJpbWl0aXZlIGluZGV4aW5nIHR5cGVzIHNwZWN1bGF0 ZXMgdGhhdCB0aGUgaW5wdXRzIGFyZSB0aGUgYXBwcm9wcmlhdGUKKyAgICAgICAgcHJpbWl0aXZl cy4gTm93LCB0aGUgQ0ZBIGZpbHRlcnMgdGhlIGFic3RyYWN0IHN0YXRlIGFjY29yZGluZ2x5LCBh cyB3ZWxsLgorCisgICAgICAgICogZGZnL0RGR0Fic3RyYWN0U3RhdGUuY3BwOgorICAgICAgICAo SlNDOjpERkc6OkFic3RyYWN0U3RhdGU6OmV4ZWN1dGUpOgorCiAyMDEzLTAyLTE0ICBHZW9mZnJl eSBHYXJlbiAgPGdnYXJlbkBhcHBsZS5jb20+CiAKICAgICAgICAgTWVyZ2VkIHRoZSBnbG9iYWwg ZnVuY3Rpb24gY2FjaGUgaW50byB0aGUgc291cmNlIGNvZGUgY2FjaGUKSW5kZXg6IFNvdXJjZS9K YXZhU2NyaXB0Q29yZS9kZmcvREZHQWJzdHJhY3RTdGF0ZS5jcHAKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL2RmZy9ERkdBYnN0cmFjdFN0YXRlLmNwcAkocmV2aXNpb24gMTQy OTY0KQorKysgU291cmNlL0phdmFTY3JpcHRDb3JlL2RmZy9ERkdBYnN0cmFjdFN0YXRlLmNwcAko d29ya2luZyBjb3B5KQpAQCAtMTI4NCw2ICsxMjg0LDI4IEBAIGJvb2wgQWJzdHJhY3RTdGF0ZTo6 ZXhlY3V0ZSh1bnNpZ25lZCBpbmQKICAgICAgICAgICAgIAogICAgIGNhc2UgTmV3QXJyYXk6CiAg ICAgICAgIG5vZGUtPnNldENhbkV4aXQodHJ1ZSk7CisgICAgICAgIHN3aXRjaCAobm9kZS0+aW5k ZXhpbmdUeXBlKCkpIHsKKyAgICAgICAgY2FzZSBBTExfQkxBTktfSU5ERVhJTkdfVFlQRVM6Cisg ICAgICAgICAgICBDUkFTSCgpOworICAgICAgICAgICAgYnJlYWs7CisgICAgICAgIGNhc2UgQUxM X1VOREVDSURFRF9JTkRFWElOR19UWVBFUzoKKyAgICAgICAgICAgIEFTU0VSVCghbm9kZS0+bnVt Q2hpbGRyZW4oKSk7CisgICAgICAgICAgICBicmVhazsKKyAgICAgICAgY2FzZSBBTExfSU5UMzJf SU5ERVhJTkdfVFlQRVM6CisgICAgICAgICAgICBmb3IgKHVuc2lnbmVkIG9wZXJhbmRJbmRleCA9 IDA7IG9wZXJhbmRJbmRleCA8IG5vZGUtPm51bUNoaWxkcmVuKCk7ICsrb3BlcmFuZEluZGV4KQor ICAgICAgICAgICAgICAgIGZvck5vZGUobV9ncmFwaC5tX3ZhckFyZ0NoaWxkcmVuW25vZGUtPmZp cnN0Q2hpbGQoKSArIG9wZXJhbmRJbmRleF0pLmZpbHRlcihTcGVjSW50MzIpOworICAgICAgICAg ICAgYnJlYWs7CisgICAgICAgIGNhc2UgQUxMX0RPVUJMRV9JTkRFWElOR19UWVBFUzoKKyAgICAg ICAgICAgIGZvciAodW5zaWduZWQgb3BlcmFuZEluZGV4ID0gMDsgb3BlcmFuZEluZGV4IDwgbm9k ZS0+bnVtQ2hpbGRyZW4oKTsgKytvcGVyYW5kSW5kZXgpCisgICAgICAgICAgICAgICAgZm9yTm9k ZShtX2dyYXBoLm1fdmFyQXJnQ2hpbGRyZW5bbm9kZS0+Zmlyc3RDaGlsZCgpICsgb3BlcmFuZElu ZGV4XSkuZmlsdGVyKFNwZWNSZWFsTnVtYmVyKTsKKyAgICAgICAgICAgIGJyZWFrOworICAgICAg ICBjYXNlIEFMTF9DT05USUdVT1VTX0lOREVYSU5HX1RZUEVTOgorICAgICAgICBjYXNlIEFMTF9B UlJBWV9TVE9SQUdFX0lOREVYSU5HX1RZUEVTOgorICAgICAgICAgICAgYnJlYWs7CisgICAgICAg IGRlZmF1bHQ6CisgICAgICAgICAgICBDUkFTSCgpOworICAgICAgICAgICAgYnJlYWs7CisgICAg ICAgIH0KICAgICAgICAgZm9yTm9kZShub2RlKS5zZXQobV9ncmFwaC5nbG9iYWxPYmplY3RGb3Io bm9kZS0+Y29kZU9yaWdpbiktPmFycmF5U3RydWN0dXJlRm9ySW5kZXhpbmdUeXBlRHVyaW5nQWxs b2NhdGlvbihub2RlLT5pbmRleGluZ1R5cGUoKSkpOwogICAgICAgICBtX2hhdmVTdHJ1Y3R1cmVz ID0gdHJ1ZTsKICAgICAgICAgYnJlYWs7Cg==