128155 2014-02-03 19:47:25 -0800 ASSERT in speculateMachineInt on 32-bit platforms 2014-02-04 11:05:14 -0800 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 mhahnenberg mhahnenberg commit-queue fpizlo oldest_to_newest 976169 0 mhahnenberg 2014-02-03 19:47:25 -0800 Patch on the way. 976170 1 223064 mhahnenberg 2014-02-03 19:49:16 -0800 Created attachment 223064 Patch 976171 2 223064 ggaren 2014-02-03 19:50:49 -0800 Comment on attachment 223064 Patch r=me 976372 3 223064 fpizlo 2014-02-04 08:48:12 -0800 Comment on attachment 223064 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=223064&action=review > Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp:139 > if (type == SpecInt52AsDouble) > - type = SpecInt52; > + type = enableInt52() ? SpecInt52 : SpecDouble; I'm pretty sure this is wrong and will result in worse optimizations on 32-bit. In particular, you're claiming that if something was representable as an in52 (i.e. it's non-fractional and not-NaN and not infinity), then it's SpecDouble, which means "this value can be an integer, or a real, or infinity, or NaN, etc". So, this is not the right fix. You should have instead just made this say: if (type == SpecInt52AsDouble && enableInt52()) type = SpecInt52; 976373 4 mhahnenberg 2014-02-04 08:49:27 -0800 > You should have instead just made this say: > > if (type == SpecInt52AsDouble && enableInt52()) > type = SpecInt52; But what happens when !enableInt52()? Is it valid for the speculated type to be SpecInt52AsDouble on 32-bit platforms? 976420 5 fpizlo 2014-02-04 10:12:47 -0800 (In reply to comment #4) > > You should have instead just made this say: > > > > if (type == SpecInt52AsDouble && enableInt52()) > > type = SpecInt52; > > But what happens when !enableInt52()? Is it valid for the speculated type to be SpecInt52AsDouble on 32-bit platforms? Yes. 976423 6 223135 mhahnenberg 2014-02-04 10:15:28 -0800 Created attachment 223135 Patch 976442 7 mhahnenberg 2014-02-04 11:05:05 -0800 Committed r163391: <http://trac.webkit.org/changeset/163391> 223064 2014-02-03 19:49:16 -0800 2014-02-04 10:15:27 -0800 Patch bug-128155-20140203194915.patch text/plain 1367 mhahnenberg SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMTYzMzU5KQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDEzIEBA CisyMDE0LTAyLTAzICBNYXJrIEhhaG5lbmJlcmcgIDxtaGFobmVuYmVyZ0BhcHBsZS5jb20+CisK KyAgICAgICAgQVNTRVJUIGluIHNwZWN1bGF0ZU1hY2hpbmVJbnQgb24gMzItYml0IHBsYXRmb3Jt cworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTI4MTU1 CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiBkZmcv REZHUHJlZGljdGlvblByb3BhZ2F0aW9uUGhhc2UuY3BwOgorICAgICAgICAoSlNDOjpERkc6OlBy ZWRpY3Rpb25Qcm9wYWdhdGlvblBoYXNlOjpwcm9wYWdhdGUpOiBDaGVjayBpZiB3ZSBzaG91bGQg YmUgdXNpbmcgSW50NTIuCisKIDIwMTQtMDItMDMgIE1hcmsgSGFobmVuYmVyZyAgPG1oYWhuZW5i ZXJnQGFwcGxlLmNvbT4KIAogICAgICAgICBMTEludDogUmVnZXggZm9yIHBzZXVkby1pbnN0cnVj dGlvbnMgaXMgdG9vIGJpZwpJbmRleDogU291cmNlL0phdmFTY3JpcHRDb3JlL2RmZy9ERkdQcmVk aWN0aW9uUHJvcGFnYXRpb25QaGFzZS5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL0phdmFTY3Jp cHRDb3JlL2RmZy9ERkdQcmVkaWN0aW9uUHJvcGFnYXRpb25QaGFzZS5jcHAJKHJldmlzaW9uIDE2 MzM1OCkKKysrIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9kZmcvREZHUHJlZGljdGlvblByb3BhZ2F0 aW9uUGhhc2UuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC0xMzYsNyArMTM2LDcgQEAgcHJpdmF0ZToK ICAgICAgICAgY2FzZSBXZWFrSlNDb25zdGFudDogewogICAgICAgICAgICAgU3BlY3VsYXRlZFR5 cGUgdHlwZSA9IHNwZWN1bGF0aW9uRnJvbVZhbHVlKG1fZ3JhcGgudmFsdWVPZkpTQ29uc3RhbnQo bm9kZSkpOwogICAgICAgICAgICAgaWYgKHR5cGUgPT0gU3BlY0ludDUyQXNEb3VibGUpCi0gICAg ICAgICAgICAgICAgdHlwZSA9IFNwZWNJbnQ1MjsKKyAgICAgICAgICAgICAgICB0eXBlID0gZW5h YmxlSW50NTIoKSA/IFNwZWNJbnQ1MiA6IFNwZWNEb3VibGU7CiAgICAgICAgICAgICBjaGFuZ2Vk IHw9IHNldFByZWRpY3Rpb24odHlwZSk7CiAgICAgICAgICAgICBicmVhazsKICAgICAgICAgfQo= 223135 2014-02-04 10:15:28 -0800 2014-02-04 11:05:14 -0800 Patch bug-128155-20140204101524.patch text/plain 1339 mhahnenberg SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMTYzMzg3KQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDEzIEBA CisyMDE0LTAyLTA0ICBNYXJrIEhhaG5lbmJlcmcgIDxtaGFobmVuYmVyZ0BhcHBsZS5jb20+CisK KyAgICAgICAgQVNTRVJUIGluIHNwZWN1bGF0ZU1hY2hpbmVJbnQgb24gMzItYml0IHBsYXRmb3Jt cworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTI4MTU1 CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiBkZmcv REZHUHJlZGljdGlvblByb3BhZ2F0aW9uUGhhc2UuY3BwOgorICAgICAgICAoSlNDOjpERkc6OlBy ZWRpY3Rpb25Qcm9wYWdhdGlvblBoYXNlOjpwcm9wYWdhdGUpOgorCiAyMDE0LTAyLTA0ICBNYXJr IEhhaG5lbmJlcmcgIDxtaGFobmVuYmVyZ0BhcHBsZS5jb20+CiAKICAgICAgICAgR0MgdGltZXIg c2hvdWxkIGFsd2F5cyBkbyBhIEZ1bGxDb2xsZWN0aW9uCkluZGV4OiBTb3VyY2UvSmF2YVNjcmlw dENvcmUvZGZnL0RGR1ByZWRpY3Rpb25Qcm9wYWdhdGlvblBoYXNlLmNwcAo9PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0t LSBTb3VyY2UvSmF2YVNjcmlwdENvcmUvZGZnL0RGR1ByZWRpY3Rpb25Qcm9wYWdhdGlvblBoYXNl LmNwcAkocmV2aXNpb24gMTYzMzU4KQorKysgU291cmNlL0phdmFTY3JpcHRDb3JlL2RmZy9ERkdQ cmVkaWN0aW9uUHJvcGFnYXRpb25QaGFzZS5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTEzNSw3ICsx MzUsNyBAQCBwcml2YXRlOgogICAgICAgICBjYXNlIEpTQ29uc3RhbnQ6CiAgICAgICAgIGNhc2Ug V2Vha0pTQ29uc3RhbnQ6IHsKICAgICAgICAgICAgIFNwZWN1bGF0ZWRUeXBlIHR5cGUgPSBzcGVj dWxhdGlvbkZyb21WYWx1ZShtX2dyYXBoLnZhbHVlT2ZKU0NvbnN0YW50KG5vZGUpKTsKLSAgICAg ICAgICAgIGlmICh0eXBlID09IFNwZWNJbnQ1MkFzRG91YmxlKQorICAgICAgICAgICAgaWYgKHR5 cGUgPT0gU3BlY0ludDUyQXNEb3VibGUgJiYgZW5hYmxlSW50NTIoKSkKICAgICAgICAgICAgICAg ICB0eXBlID0gU3BlY0ludDUyOwogICAgICAgICAgICAgY2hhbmdlZCB8PSBzZXRQcmVkaWN0aW9u KHR5cGUpOwogICAgICAgICAgICAgYnJlYWs7Cg==