124087 2013-11-08 16:57:19 -0800 Get rid of the bizarre Darwin/x86-only MacroAssembler::shouldBlindForSpecificArch(uintptr_t) overload 2013-11-08 17:41:29 -0800 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 1 fpizlo fpizlo aestes barraclough ggaren mark.lam mhahnenberg msaboff oliver sam oldest_to_newest 948939 0 fpizlo 2013-11-08 16:57:19 -0800 Patch forthcoming. 948940 1 216464 fpizlo 2013-11-08 16:57:53 -0800 Created attachment 216464 the patch 948951 2 fpizlo 2013-11-08 17:27:51 -0800 Landed in http://trac.webkit.org/changeset/158981 948957 3 216464 darin 2013-11-08 17:36:58 -0800 Comment on attachment 216464 the patch View in context: https://bugs.webkit.org/attachment.cgi?id=216464&action=review > Source/JavaScriptCore/assembler/MacroAssembler.h:992 > + if (sizeof(void*) == 4) > + return shouldBlindForSpecificArch(static_cast<uint32_t>(value)); > + return shouldBlindForSpecificArch(static_cast<uint64_t>(value)); Why doesn’t this work? return shouldBlindForSpecificArch(value); I’d expect that overloading would take care of the rest. No need for the if statement. 948960 4 216464 fpizlo 2013-11-08 17:41:29 -0800 Comment on attachment 216464 the patch View in context: https://bugs.webkit.org/attachment.cgi?id=216464&action=review >> Source/JavaScriptCore/assembler/MacroAssembler.h:992 >> + return shouldBlindForSpecificArch(static_cast<uint64_t>(value)); > > Why doesn’t this work? > > return shouldBlindForSpecificArch(value); > > I’d expect that overloading would take care of the rest. No need for the if statement. On some compilers (including a version of clang in one of the toolchains that we build on), uint32_t, uint64_t and uintptr_t are different types. Hence passing uintptr_t causes a compile error; the compiler claims that the call overload is ambiguous. This patch fixes that compile error. It also removes a Darwin-only, and x86-only hack to avoid this ambiguity. See the code I remove in MacroAssemblerX86Common. 216464 2013-11-08 16:57:53 -0800 2013-11-08 17:41:29 -0800 the patch blah.patch text/plain 2611 fpizlo SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMTU4OTc4KQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE2IEBA CisyMDEzLTExLTA4ICBGaWxpcCBQaXpsbyAgPGZwaXpsb0BhcHBsZS5jb20+CisKKyAgICAgICAg R2V0IHJpZCBvZiB0aGUgYml6YXJyZSBEYXJ3aW4veDg2LW9ubHkgTWFjcm9Bc3NlbWJsZXI6OnNo b3VsZEJsaW5kRm9yU3BlY2lmaWNBcmNoKHVpbnRwdHJfdCkgb3ZlcmxvYWQKKyAgICAgICAgaHR0 cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTEyNDA4NworCisgICAgICAgIFJl dmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgICogYXNzZW1ibGVyL01hY3JvQXNz ZW1ibGVyLmg6CisgICAgICAgIChKU0M6Ok1hY3JvQXNzZW1ibGVyOjpzaG91bGRCbGluZFBvaW50 ZXJGb3JTcGVjaWZpY0FyY2gpOgorICAgICAgICAoSlNDOjpNYWNyb0Fzc2VtYmxlcjo6c2hvdWxk QmxpbmQpOgorICAgICAgICAqIGFzc2VtYmxlci9NYWNyb0Fzc2VtYmxlclg4NkNvbW1vbi5oOgor ICAgICAgICAoSlNDOjpNYWNyb0Fzc2VtYmxlclg4NkNvbW1vbjo6c2hvdWxkQmxpbmRGb3JTcGVj aWZpY0FyY2gpOgorCiAyMDEzLTExLTA4ICBGaWxpcCBQaXpsbyAgPGZwaXpsb0BhcHBsZS5jb20+ CiAKICAgICAgICAgRlRMIHNob3VsZCBzdXBwb3J0IE5ld0FycmF5QnVmZmVyCkluZGV4OiBTb3Vy Y2UvSmF2YVNjcmlwdENvcmUvYXNzZW1ibGVyL01hY3JvQXNzZW1ibGVyLmgKPT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQot LS0gU291cmNlL0phdmFTY3JpcHRDb3JlL2Fzc2VtYmxlci9NYWNyb0Fzc2VtYmxlci5oCShyZXZp c2lvbiAxNTg5NzUpCisrKyBTb3VyY2UvSmF2YVNjcmlwdENvcmUvYXNzZW1ibGVyL01hY3JvQXNz ZW1ibGVyLmgJKHdvcmtpbmcgY29weSkKQEAgLTk4NSw2ICs5ODUsMTMgQEAgcHVibGljOgogICAg ICAgICByZXR1cm4gdmFsdWUgPiAweGZmOwogICAgIH0KICAgICAKKyAgICBib29sIHNob3VsZEJs aW5kUG9pbnRlckZvclNwZWNpZmljQXJjaCh1aW50cHRyX3QgdmFsdWUpCisgICAgeworICAgICAg ICBpZiAoc2l6ZW9mKHZvaWQqKSA9PSA0KQorICAgICAgICAgICAgcmV0dXJuIHNob3VsZEJsaW5k Rm9yU3BlY2lmaWNBcmNoKHN0YXRpY19jYXN0PHVpbnQzMl90Pih2YWx1ZSkpOworICAgICAgICBy ZXR1cm4gc2hvdWxkQmxpbmRGb3JTcGVjaWZpY0FyY2goc3RhdGljX2Nhc3Q8dWludDY0X3Q+KHZh bHVlKSk7CisgICAgfQorICAgIAogICAgIGJvb2wgc2hvdWxkQmxpbmQoSW1tUHRyIGltbSkKICAg ICB7CiAgICAgICAgIGlmICghY2FuQmxpbmQoKSkKQEAgLTEwMjAsNyArMTAyNyw3IEBAIHB1Ymxp YzoKICAgICAgICAgaWYgKCFzaG91bGRDb25zaWRlckJsaW5kaW5nKCkpCiAgICAgICAgICAgICBy ZXR1cm4gZmFsc2U7CiAKLSAgICAgICAgcmV0dXJuIHNob3VsZEJsaW5kRm9yU3BlY2lmaWNBcmNo KHZhbHVlKTsKKyAgICAgICAgcmV0dXJuIHNob3VsZEJsaW5kUG9pbnRlckZvclNwZWNpZmljQXJj aCh2YWx1ZSk7CiAgICAgfQogICAgIAogICAgIHN0cnVjdCBSb3RhdGVkSW1tUHRyIHsKSW5kZXg6 IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9hc3NlbWJsZXIvTWFjcm9Bc3NlbWJsZXJYODZDb21tb24u aAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09Ci0tLSBTb3VyY2UvSmF2YVNjcmlwdENvcmUvYXNzZW1ibGVyL01hY3JvQXNz ZW1ibGVyWDg2Q29tbW9uLmgJKHJldmlzaW9uIDE1ODk3NSkKKysrIFNvdXJjZS9KYXZhU2NyaXB0 Q29yZS9hc3NlbWJsZXIvTWFjcm9Bc3NlbWJsZXJYODZDb21tb24uaAkod29ya2luZyBjb3B5KQpA QCAtOTcsMTIgKzk3LDcgQEAgcHVibGljOgogICAgIAogICAgIHN0YXRpYyBib29sIGNhbkJsaW5k KCkgeyByZXR1cm4gdHJ1ZTsgfQogICAgIHN0YXRpYyBib29sIHNob3VsZEJsaW5kRm9yU3BlY2lm aWNBcmNoKHVpbnQzMl90IHZhbHVlKSB7IHJldHVybiB2YWx1ZSA+PSAweDAwZmZmZmZmOyB9Ci0j aWYgQ1BVKFg4Nl82NCkKICAgICBzdGF0aWMgYm9vbCBzaG91bGRCbGluZEZvclNwZWNpZmljQXJj aCh1aW50NjRfdCB2YWx1ZSkgeyByZXR1cm4gdmFsdWUgPj0gMHgwMGZmZmZmZjsgfQotI2lmIE9T KERBUldJTikgLy8gT24gNjQtYml0IHN5c3RlbXMgb3RoZXIgdGhhbiBEQVJXSU4gdWludDY0X3Qg YW5kIHVpbnRwdHJfdCBhcmUgdGhlIHNhbWUgdHlwZSBzbyBvdmVybG9hZCBpcyBwcm9oaWJpdGVk LgotICAgIHN0YXRpYyBib29sIHNob3VsZEJsaW5kRm9yU3BlY2lmaWNBcmNoKHVpbnRwdHJfdCB2 YWx1ZSkgeyByZXR1cm4gdmFsdWUgPj0gMHgwMGZmZmZmZjsgfQotI2VuZGlmCi0jZW5kaWYKIAog ICAgIC8vIEludGVnZXIgYXJpdGhtZXRpYyBvcGVyYXRpb25zOgogICAgIC8vCg==