49112 2010-11-05 16:35:00 -0700 Fix Accelerate.framework issues in VectorMath 32-bit ppc & i386 vs. other architectures (64-bit, ARM, etc.) 2010-11-11 18:49:49 -0800 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Other OS X 10.5 RESOLVED FIXED P2 Normal --- 0 crogers webkit-unassigned cmarrin commit-queue dglazkov eric.carlson jamesr jer.noble kbr simon.fraser oldest_to_newest 305627 0 crogers 2010-11-05 16:35:00 -0700 Fix 32-bit vs. 64-bit Accelerate.framework issues in VectorMath 305628 1 73139 crogers 2010-11-05 16:37:30 -0700 Created attachment 73139 Patch 306044 2 73139 mjs 2010-11-08 00:43:05 -0800 Comment on attachment 73139 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=73139&action=review What is the effect of this patch? Does the current code fail to build on 32-bit, or does it build but behave differently when changed this way? Is there a reason we can't use the same code path for both 32-bit and 64-bit platforms? Also of note: ARM is a 32-bit platform not covered by your macros. Is it ok for it to go down the same code path as 64-bit? I think either your macro test is wrong (if ARM should be included with 32-bit)) or the ChangeLog is wrong (if the issue is specific to just i386 and ppc, rather than all 32-bit platforms). > WebCore/platform/audio/VectorMath.cpp:41 > +// In 32-bit mode __ppc__ or __i386__ is defined and <vecLib/vDSP_translate.h> is included which defines macros, so we must handle this case differently. This comment does not seem useful. It says the 32-bit case is handled differently, but does not say why. 306189 3 crogers 2010-11-08 10:21:09 -0800 Maciej, thanks for having a look. The current code builds on 32-bit but goes into a loop and crashes when vadd() calls back into itself recursively because of a macro which is defined only in the __ppc__ and __i386__ case. The vDSP_translate.h file defines these macros, but only gets included in the 32-bit case. Even though my vadd() and vsmul() functions are in a namespace, the macro screws things up, and it seems unfortunate that Accelerate.h is defining these kinds of macros. The ARM case should be OK, since I'm using exactly the same compile-time conditional as the framework is using. 306267 4 73261 crogers 2010-11-08 12:00:30 -0800 Created attachment 73261 Patch 306268 5 crogers 2010-11-08 12:01:29 -0800 Changed name of this bug and uploaded a new patch with a more descriptive comment. 308177 6 73261 kbr 2010-11-11 14:50:03 -0800 Comment on attachment 73261 Patch Looks good to me. 308323 7 73261 commit-queue 2010-11-11 18:49:43 -0800 Comment on attachment 73261 Patch Clearing flags on attachment: 73261 Committed r71877: <http://trac.webkit.org/changeset/71877> 308324 8 commit-queue 2010-11-11 18:49:49 -0800 All reviewed patches have been landed. Closing bug. 73139 2010-11-05 16:37:30 -0700 2010-11-08 12:00:26 -0800 Patch bug-49112-20101105163729.patch text/plain 1973 crogers SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA3MTQ1NykKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTYgQEAKKzIwMTAtMTEtMDUgIENocmlzIFJvZ2VycyAgPGNyb2dlcnNAZ29vZ2xl LmNvbT4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBG aXggMzItYml0IHZzLiA2NC1iaXQgQWNjZWxlcmF0ZS5mcmFtZXdvcmsgaXNzdWVzIGluIFZlY3Rv ck1hdGgKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTQ5 MTEyCisKKyAgICAgICAgTm8gbmV3IHRlc3RzIHNpbmNlIGF1ZGlvIEFQSSBpcyBub3QgeWV0IGlt cGxlbWVudGVkLgorCisgICAgICAgICogcGxhdGZvcm0vYXVkaW8vVmVjdG9yTWF0aC5jcHA6Cisg ICAgICAgIChXZWJDb3JlOjpWZWN0b3JNYXRoOjp2c211bCk6CisgICAgICAgIChXZWJDb3JlOjpW ZWN0b3JNYXRoOjp2YWRkKToKKwogMjAxMC0xMS0wNSAgUnlvc3VrZSBOaXdhICA8cm5pd2FAd2Vi a2l0Lm9yZz4KIAogICAgICAgICBSZXZpZXdlZCBieSBEYXZpZCBMZXZpbi4KSW5kZXg6IFdlYkNv cmUvcGxhdGZvcm0vYXVkaW8vVmVjdG9yTWF0aC5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gV2ViQ29yZS9w bGF0Zm9ybS9hdWRpby9WZWN0b3JNYXRoLmNwcAkocmV2aXNpb24gNzE0NDYpCisrKyBXZWJDb3Jl L3BsYXRmb3JtL2F1ZGlvL1ZlY3Rvck1hdGguY3BwCSh3b3JraW5nIGNvcHkpCkBAIC0zOCwxNSAr MzgsMjQgQEAgbmFtZXNwYWNlIFZlY3Rvck1hdGggewogCiAjaWYgT1MoREFSV0lOKQogLy8gT24g dGhlIE1hYyB3ZSB1c2UgdGhlIGhpZ2hseSBvcHRpbWl6ZWQgdmVyc2lvbnMgaW4gQWNjZWxlcmF0 ZS5mcmFtZXdvcmsKKy8vIEluIDMyLWJpdCBtb2RlIF9fcHBjX18gb3IgX19pMzg2X18gaXMgZGVm aW5lZCBhbmQgPHZlY0xpYi92RFNQX3RyYW5zbGF0ZS5oPiBpcyBpbmNsdWRlZCB3aGljaCBkZWZp bmVzIG1hY3Jvcywgc28gd2UgbXVzdCBoYW5kbGUgdGhpcyBjYXNlIGRpZmZlcmVudGx5LgogCiB2 b2lkIHZzbXVsKGNvbnN0IGZsb2F0KiBzb3VyY2VQLCBpbnQgc291cmNlU3RyaWRlLCBjb25zdCBm bG9hdCogc2NhbGUsIGZsb2F0KiBkZXN0UCwgaW50IGRlc3RTdHJpZGUsIHNpemVfdCBmcmFtZXNU b1Byb2Nlc3MpCiB7CisjaWYgZGVmaW5lZChfX3BwY19fKSB8fCBkZWZpbmVkKF9faTM4Nl9fKQor ICAgIDo6dnNtdWwoc291cmNlUCwgc291cmNlU3RyaWRlLCBzY2FsZSwgZGVzdFAsIGRlc3RTdHJp ZGUsIGZyYW1lc1RvUHJvY2Vzcyk7CisjZWxzZQogICAgIHZEU1BfdnNtdWwoc291cmNlUCwgc291 cmNlU3RyaWRlLCBzY2FsZSwgZGVzdFAsIGRlc3RTdHJpZGUsIGZyYW1lc1RvUHJvY2Vzcyk7Cisj ZW5kaWYKIH0KIAogdm9pZCB2YWRkKGNvbnN0IGZsb2F0KiBzb3VyY2UxUCwgaW50IHNvdXJjZVN0 cmlkZTEsIGNvbnN0IGZsb2F0KiBzb3VyY2UyUCwgaW50IHNvdXJjZVN0cmlkZTIsIGZsb2F0KiBk ZXN0UCwgaW50IGRlc3RTdHJpZGUsIHNpemVfdCBmcmFtZXNUb1Byb2Nlc3MpCiB7CisjaWYgZGVm aW5lZChfX3BwY19fKSB8fCBkZWZpbmVkKF9faTM4Nl9fKQorICAgIDo6dmFkZChzb3VyY2UxUCwg c291cmNlU3RyaWRlMSwgc291cmNlMlAsIHNvdXJjZVN0cmlkZTIsIGRlc3RQLCBkZXN0U3RyaWRl LCBmcmFtZXNUb1Byb2Nlc3MpOworI2Vsc2UKICAgICB2RFNQX3ZhZGQoc291cmNlMVAsIHNvdXJj ZVN0cmlkZTEsIHNvdXJjZTJQLCBzb3VyY2VTdHJpZGUyLCBkZXN0UCwgZGVzdFN0cmlkZSwgZnJh bWVzVG9Qcm9jZXNzKTsKKyNlbmRpZgogfQogCiAjZWxzZQo= 73261 2010-11-08 12:00:30 -0800 2010-11-11 18:49:43 -0800 Patch bug-49112-20101108120033.patch text/plain 2564 crogers SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA3MTU1MykKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMjEgQEAKKzIwMTAtMTEtMDggIENocmlzIFJvZ2VycyAgPGNyb2dlcnNAZ29vZ2xl LmNvbT4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBG aXggQWNjZWxlcmF0ZS5mcmFtZXdvcmsgaXNzdWVzIGluIFZlY3Rvck1hdGggMzItYml0IHBwYyAm IGkzODYgdnMuIG90aGVyIGFyY2hpdGVjdHVyZXMgKDY0LWJpdCwgQVJNLCBldGMuKS4KKyAgICAg ICAgT24gdGhlIE1hYyB3ZSB1c2UgdGhlIGhpZ2hseSBvcHRpbWl6ZWQgdmVyc2lvbnMgaW4gQWNj ZWxlcmF0ZS5mcmFtZXdvcmsKKyAgICAgICAgSW4gMzItYml0IG1vZGUgKF9fcHBjX18gb3IgX19p Mzg2X18pIDxBY2NlbGVyYXRlL0FjY2VsZXJhdGUuaD4gaW5jbHVkZXMgPHZlY0xpYi92RFNQX3Ry YW5zbGF0ZS5oPiB3aGljaCBkZWZpbmVzIG1hY3JvcworICAgICAgICBvZiB0aGUgc2FtZSBuYW1l IGFzIG91ciBuYW1lc3BhY2VkIGZ1bmN0aW9uIG5hbWVzLCBzbyB3ZSBtdXN0IGhhbmRsZSB0aGlz IGNhc2UgZGlmZmVyZW50bHkuCisgICAgICAgIE90aGVyIGFyY2hpdGVjdHVyZXMgKDY0Yml0LCBB Uk0sIGV0Yy4pIGRvIG5vdCBpbmNsdWRlIHRoaXMgaGVhZGVyIGZpbGUuCisgICAgICAgIAorICAg ICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NDkxMTIKKworICAg ICAgICBObyBuZXcgdGVzdHMgc2luY2UgYXVkaW8gQVBJIGlzIG5vdCB5ZXQgaW1wbGVtZW50ZWQu CisKKyAgICAgICAgKiBwbGF0Zm9ybS9hdWRpby9WZWN0b3JNYXRoLmNwcDoKKyAgICAgICAgKFdl YkNvcmU6OlZlY3Rvck1hdGg6OnZzbXVsKToKKyAgICAgICAgKFdlYkNvcmU6OlZlY3Rvck1hdGg6 OnZhZGQpOgorCiAyMDEwLTExLTA4ICBTaW1vbiBGcmFzZXIgIDxzaW1vbi5mcmFzZXJAYXBwbGUu Y29tPgogCiAgICAgICAgIFJldmlld2VkIGJ5IERhbiBCZXJuc3RlaW4uCkluZGV4OiBXZWJDb3Jl L3BsYXRmb3JtL2F1ZGlvL1ZlY3Rvck1hdGguY3BwCj09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvcGxh dGZvcm0vYXVkaW8vVmVjdG9yTWF0aC5jcHAJKHJldmlzaW9uIDcxNDU3KQorKysgV2ViQ29yZS9w bGF0Zm9ybS9hdWRpby9WZWN0b3JNYXRoLmNwcAkod29ya2luZyBjb3B5KQpAQCAtMzgsMTUgKzM4 LDI1IEBAIG5hbWVzcGFjZSBWZWN0b3JNYXRoIHsKIAogI2lmIE9TKERBUldJTikKIC8vIE9uIHRo ZSBNYWMgd2UgdXNlIHRoZSBoaWdobHkgb3B0aW1pemVkIHZlcnNpb25zIGluIEFjY2VsZXJhdGUu ZnJhbWV3b3JrCisvLyBJbiAzMi1iaXQgbW9kZSAoX19wcGNfXyBvciBfX2kzODZfXykgPEFjY2Vs ZXJhdGUvQWNjZWxlcmF0ZS5oPiBpbmNsdWRlcyA8dmVjTGliL3ZEU1BfdHJhbnNsYXRlLmg+IHdo aWNoIGRlZmluZXMgbWFjcm9zIG9mIHRoZSBzYW1lIG5hbWUgYXMKKy8vIG91ciBuYW1lc3BhY2Vk IGZ1bmN0aW9uIG5hbWVzLCBzbyB3ZSBtdXN0IGhhbmRsZSB0aGlzIGNhc2UgZGlmZmVyZW50bHku ICBPdGhlciBhcmNoaXRlY3R1cmVzICg2NGJpdCwgQVJNLCBldGMuKSBkbyBub3QgaW5jbHVkZSB0 aGlzIGhlYWRlciBmaWxlLgogCiB2b2lkIHZzbXVsKGNvbnN0IGZsb2F0KiBzb3VyY2VQLCBpbnQg c291cmNlU3RyaWRlLCBjb25zdCBmbG9hdCogc2NhbGUsIGZsb2F0KiBkZXN0UCwgaW50IGRlc3RT dHJpZGUsIHNpemVfdCBmcmFtZXNUb1Byb2Nlc3MpCiB7CisjaWYgZGVmaW5lZChfX3BwY19fKSB8 fCBkZWZpbmVkKF9faTM4Nl9fKQorICAgIDo6dnNtdWwoc291cmNlUCwgc291cmNlU3RyaWRlLCBz Y2FsZSwgZGVzdFAsIGRlc3RTdHJpZGUsIGZyYW1lc1RvUHJvY2Vzcyk7CisjZWxzZQogICAgIHZE U1BfdnNtdWwoc291cmNlUCwgc291cmNlU3RyaWRlLCBzY2FsZSwgZGVzdFAsIGRlc3RTdHJpZGUs IGZyYW1lc1RvUHJvY2Vzcyk7CisjZW5kaWYKIH0KIAogdm9pZCB2YWRkKGNvbnN0IGZsb2F0KiBz b3VyY2UxUCwgaW50IHNvdXJjZVN0cmlkZTEsIGNvbnN0IGZsb2F0KiBzb3VyY2UyUCwgaW50IHNv dXJjZVN0cmlkZTIsIGZsb2F0KiBkZXN0UCwgaW50IGRlc3RTdHJpZGUsIHNpemVfdCBmcmFtZXNU b1Byb2Nlc3MpCiB7CisjaWYgZGVmaW5lZChfX3BwY19fKSB8fCBkZWZpbmVkKF9faTM4Nl9fKQor ICAgIDo6dmFkZChzb3VyY2UxUCwgc291cmNlU3RyaWRlMSwgc291cmNlMlAsIHNvdXJjZVN0cmlk ZTIsIGRlc3RQLCBkZXN0U3RyaWRlLCBmcmFtZXNUb1Byb2Nlc3MpOworI2Vsc2UKICAgICB2RFNQ X3ZhZGQoc291cmNlMVAsIHNvdXJjZVN0cmlkZTEsIHNvdXJjZTJQLCBzb3VyY2VTdHJpZGUyLCBk ZXN0UCwgZGVzdFN0cmlkZSwgZnJhbWVzVG9Qcm9jZXNzKTsKKyNlbmRpZgogfQogCiAjZWxzZQo=