16925 2008-01-18 13:18:35 -0800 How to fix VectorBuffer's alignment FIXME 2008-09-21 13:18:25 -0700 1 1 1 Unclassified WebKit Web Template Framework 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 1 maruel webkit-unassigned ap ddkilzer ggaren mjs oldest_to_newest 67612 0 maruel 2008-01-18 13:18:35 -0800 To remove the FIXME for VectorBuffer<>::m_inlineBuffer at line 381, you should define the alignment, which is a compiler-specific attribute. That would help for Vector<double> and would be needed to use with SSE primitives. For GCC, I think that would be: char __attribute__((aligned(__alignof__(T)))) m_inlineBuffer[m_inlineBufferSize]; For MSVC, I think that would be: char __declspec(align(__alignof(T))) m_inlineBuffer[m_inlineBufferSize]; For sure, a macro would need to be defined to not directly clutter the code. Since I don't know where to put the necessary macros, I'll let more experimented folks figure that out. 85455 1 ap 2008-07-08 08:43:44 -0700 See also: bug 19775. 90399 2 mjs 2008-09-07 04:56:19 -0700 I think more portable than this approach would be to union the char array with a one-element array of type T. 90400 3 mjs 2008-09-07 04:58:26 -0700 Oliver reminds me that this doesn't work if T has a constructor. alignof it is then. 91340 4 23467 ppedriana 2008-09-16 02:02:40 -0700 Created attachment 23467 Patch which resolves this problem. This was recently discussed on the webkit-dev mailing list at a thread starting here: https://lists.webkit.org/pipermail/webkit-dev/2008-September/004777.html 91374 5 23467 darin 2008-09-16 09:51:16 -0700 Comment on attachment 23467 Patch which resolves this problem. Looks good! Thanks for tackling this. +#define WTF_ALIGN_OF(type) __alignof__(type) +#define WTF_ALIGNED(variable_type, variable, n) variable_type variable __attribute__((aligned(n))) These macros are fine, but Platform.h is the wrong header for them. It's a configuration header only and not the place to put macros. We could add a new header like <wtf/AlwaysInline.h> or find some other header to put them in. For now, it would be fine for them to be in Vector.h with the intention of moving them elsewhere later. + typedef char aligned_buffer_char; This is the wrong naming style for WebKit code. The type should be named something like AlignedBufferChar. + template <size_t size, size_t alignment> + struct aligned_buffer { aligned_buffer_char buffer[size]; }; Same comment here. We should fix the "wrong header" and naming issues, and then someone should test to be sure this doesn't slow down performance, and then we can land this. 91400 6 ddkilzer 2008-09-16 10:53:36 -0700 (In reply to comment #4) > Created an attachment (id=23467) [edit] > Patch which resolves this problem. A couple additional comments: 1. Please use the COMPILER() macros when checking for compilers (except for version checks). A new macro will need to be added for Metrowerks. 2. I think alignment macros need to be added for the mingw compiler before the patch lands. I think one of the existing ports uses it (Qt for Windows?), and it would break their build if it was not supported. 91437 7 23487 ppedriana 2008-09-16 14:18:06 -0700 Created attachment 23487 Revised patch This patch: - moves the align macros back to vector.h - renames aligned_buffer_char to AlignedBufferChar - renamed aligned_buffer to aligned_buffer - used #if COMPILER(GCC) instead of #if defined(__GNUC___) - adds COMPILER(MINGW) - removes the reference to __MWERKS__ for now. 92010 8 23487 darin 2008-09-21 13:18:05 -0700 Comment on attachment 23487 Revised patch r=me 92011 9 darin 2008-09-21 13:18:25 -0700 http://trac.webkit.org/changeset/36740 23467 2008-09-16 02:02:40 -0700 2008-09-16 14:18:06 -0700 Patch which resolves this problem. VectorAlignmentPatch.txt text/plain 3344 ppedriana SW5kZXg6IEphdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBKYXZhU2NyaXB0 Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDM2NDg4KQorKysgSmF2YVNjcmlwdENvcmUvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTMgQEAKKzIwMDgtMDktMTYgIFBhdWwgIDx3 ZWJraXRAcGVkcmlhbmEuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEp LgorCisgICAgICAgIEZpeGVkIGxhY2sgb2YgVmVjdG9yIGJ1ZmZlciBhbGlnbm1lbnQgaW4gYSBw b3J0YWJsZSB3YXkuCisgICAgICAgIEluY2x1ZGVzIGFkZGl0aW9uIG9mIGFsaWdubWVudCBkaXJl Y3RpdmVzIHRvIFBsYXRmb3JtLmguIAorCisgICAgICAgICogd3RmL1BsYXRmb3JtLmg6CisgICAg ICAgICogd3RmL1ZlY3Rvci5oOgorCiAyMDA4LTA5LTE1ICBNYWNpZWogU3RhY2hvd2lhayAgPG1q c0BhcHBsZS5jb20+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgQ2FtZXJvbiBad2FyaWNoLgpJbmRl eDogSmF2YVNjcmlwdENvcmUvd3RmL1BsYXRmb3JtLmgKPT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gSmF2YVNjcmlw dENvcmUvd3RmL1BsYXRmb3JtLmgJKHJldmlzaW9uIDM2NDAwKQorKysgSmF2YVNjcmlwdENvcmUv d3RmL1BsYXRmb3JtLmgJKHdvcmtpbmcgY29weSkKQEAgLTM1Nyw0ICszNTcsMTYgQEAKICNkZWZp bmUgRU5BQkxFX01BU00gMQogI2VuZGlmCiAKKy8vIFdURl9BTElHTl9PRiAvIFdURl9BTElHTkVE CisjaWYgZGVmaW5lZChfX0dOVUNfXykgfHwgZGVmaW5lZChfX01XRVJLU19fKQorI2RlZmluZSBX VEZfQUxJR05fT0YodHlwZSkgICBfX2FsaWdub2ZfXyh0eXBlKQorI2RlZmluZSBXVEZfQUxJR05F RCh2YXJpYWJsZV90eXBlLCB2YXJpYWJsZSwgbikgdmFyaWFibGVfdHlwZSB2YXJpYWJsZSBfX2F0 dHJpYnV0ZV9fKChhbGlnbmVkKG4pKSkKKyNlbGlmIGRlZmluZWQoX01TQ19WRVIpCisjZGVmaW5l IFdURl9BTElHTl9PRih0eXBlKSAgIF9fYWxpZ25vZih0eXBlKQorI2RlZmluZSBXVEZfQUxJR05F RCh2YXJpYWJsZV90eXBlLCB2YXJpYWJsZSwgbikgX19kZWNsc3BlYyhhbGlnbihuKSkgdmFyaWFi bGVfdHlwZSB2YXJpYWJsZQorI2Vsc2UKKyNlcnJvciBuZWVkIGFsaWdubWVudCBjb250cm9sCisj ZW5kaWYKKworCiAjZW5kaWYgLyogV1RGX1BsYXRmb3JtX2ggKi8KSW5kZXg6IEphdmFTY3JpcHRD b3JlL3d0Zi9WZWN0b3IuaAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBKYXZhU2NyaXB0Q29yZS93dGYvVmVjdG9y LmgJKHJldmlzaW9uIDM2NDAwKQorKysgSmF2YVNjcmlwdENvcmUvd3RmL1ZlY3Rvci5oCSh3b3Jr aW5nIGNvcHkpCkBAIC0zNiw2ICszNiwzMyBAQCBuYW1lc3BhY2UgV1RGIHsKICAgICB1c2luZyBz dGQ6Om1pbjsKICAgICB1c2luZyBzdGQ6Om1heDsKICAgICAKKyAgICAjaWYgZGVmaW5lZChfX0dO VUNfXykgJiYgKCgoX19HTlVDX18gKiAxMDApICsgX19HTlVDX01JTk9SX18pID49IDMwMykKKyAg ICAgICAgdHlwZWRlZiBjaGFyIF9fYXR0cmlidXRlX18oKF9fbWF5X2FsaWFzX18pKSBhbGlnbmVk X2J1ZmZlcl9jaGFyOyAKKyAgICAjZWxzZQorICAgICAgICB0eXBlZGVmIGNoYXIgYWxpZ25lZF9i dWZmZXJfY2hhcjsgCisgICAgI2VuZGlmCisKKyAgICB0ZW1wbGF0ZSA8c2l6ZV90IHNpemUsIHNp emVfdCBhbGlnbm1lbnQ+CisgICAgc3RydWN0IGFsaWduZWRfYnVmZmVyIHsgYWxpZ25lZF9idWZm ZXJfY2hhciBidWZmZXJbc2l6ZV07IH07CisKKyAgICB0ZW1wbGF0ZTxzaXplX3Qgc2l6ZT4KKyAg ICBzdHJ1Y3QgYWxpZ25lZF9idWZmZXI8c2l6ZSwgMj4gICAgeyBXVEZfQUxJR05FRChhbGlnbmVk X2J1ZmZlcl9jaGFyLCBidWZmZXJbc2l6ZV0sIDIpOyAgfTsKKworICAgIHRlbXBsYXRlPHNpemVf dCBzaXplPgorICAgIHN0cnVjdCBhbGlnbmVkX2J1ZmZlcjxzaXplLCA0PiAgICB7IFdURl9BTElH TkVEKGFsaWduZWRfYnVmZmVyX2NoYXIsIGJ1ZmZlcltzaXplXSwgNCk7ICB9OworCisgICAgdGVt cGxhdGU8c2l6ZV90IHNpemU+CisgICAgc3RydWN0IGFsaWduZWRfYnVmZmVyPHNpemUsIDg+ICAg IHsgV1RGX0FMSUdORUQoYWxpZ25lZF9idWZmZXJfY2hhciwgYnVmZmVyW3NpemVdLCA4KTsgIH07 CisKKyAgICB0ZW1wbGF0ZTxzaXplX3Qgc2l6ZT4KKyAgICBzdHJ1Y3QgYWxpZ25lZF9idWZmZXI8 c2l6ZSwgMTY+ICAgeyBXVEZfQUxJR05FRChhbGlnbmVkX2J1ZmZlcl9jaGFyLCBidWZmZXJbc2l6 ZV0sIDE2KTsgfTsKKworICAgIHRlbXBsYXRlPHNpemVfdCBzaXplPgorICAgIHN0cnVjdCBhbGln bmVkX2J1ZmZlcjxzaXplLCAzMj4gICB7IFdURl9BTElHTkVEKGFsaWduZWRfYnVmZmVyX2NoYXIs IGJ1ZmZlcltzaXplXSwgMzIpOyB9OworCisgICAgdGVtcGxhdGU8c2l6ZV90IHNpemU+CisgICAg c3RydWN0IGFsaWduZWRfYnVmZmVyPHNpemUsIDY0PiAgIHsgV1RGX0FMSUdORUQoYWxpZ25lZF9i dWZmZXJfY2hhciwgYnVmZmVyW3NpemVdLCA2NCk7IH07CisKICAgICB0ZW1wbGF0ZSA8Ym9vbCBu ZWVkc0Rlc3RydWN0aW9uLCB0eXBlbmFtZSBUPgogICAgIGNsYXNzIFZlY3RvckRlc3RydWN0b3I7 CiAKQEAgLTM4NCwxMCArNDExLDkgQEAgbmFtZXNwYWNlIFdURiB7CiAgICAgICAgIHVzaW5nIEJh c2U6Om1fY2FwYWNpdHk7CiAKICAgICAgICAgc3RhdGljIGNvbnN0IHNpemVfdCBtX2lubGluZUJ1 ZmZlclNpemUgPSBpbmxpbmVDYXBhY2l0eSAqIHNpemVvZihUKTsKLSAgICAgICAgVCogaW5saW5l QnVmZmVyKCkgeyByZXR1cm4gcmVpbnRlcnByZXRfY2FzdDxUKj4oJm1faW5saW5lQnVmZmVyKTsg fQorICAgICAgICBUKiBpbmxpbmVCdWZmZXIoKSB7IHJldHVybiByZWludGVycHJldF9jYXN0PFQq PihtX2lubGluZUJ1ZmZlci5idWZmZXIpOyB9CiAKLSAgICAgICAgLy8gRklYTUU6IE5vdGhpbmcg Z3VhcmFudGVlcyB0aGlzIGJ1ZmZlciBpcyBhcHByb3ByaWF0ZWx5IGFsaWduZWQgdG8gaG9sZCBv YmplY3RzIG9mIHR5cGUgVC4KLSAgICAgICAgY2hhciBtX2lubGluZUJ1ZmZlclttX2lubGluZUJ1 ZmZlclNpemVdOworICAgICAgICBhbGlnbmVkX2J1ZmZlcjxtX2lubGluZUJ1ZmZlclNpemUsIFdU Rl9BTElHTl9PRihUKT4gbV9pbmxpbmVCdWZmZXI7CiAgICAgfTsKIAogICAgIHRlbXBsYXRlPHR5 cGVuYW1lIFQsIHNpemVfdCBpbmxpbmVDYXBhY2l0eSA9IDA+Cgo= 23487 2008-09-16 14:18:06 -0700 2008-09-21 13:18:05 -0700 Revised patch VectorAlignmentPatch2.txt text/plain 2958 ppedriana SW5kZXg6IEphdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBKYXZhU2NyaXB0 Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDM2NDg4KQorKysgSmF2YVNjcmlwdENvcmUvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTEgQEAKKzIwMDgtMDktMTYgIFBhdWwgIDx3 ZWJraXRAcGVkcmlhbmEuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEp LgorCisgICAgICAgIEZpeGVkIGxhY2sgb2YgVmVjdG9yIGJ1ZmZlciBhbGlnbm1lbnQgaW4gYSBw b3J0YWJsZSB3YXkuCisKKyAgICAgICAgKiB3dGYvVmVjdG9yLmg6CisKIDIwMDgtMDktMTUgIE1h Y2llaiBTdGFjaG93aWFrICA8bWpzQGFwcGxlLmNvbT4KIAogICAgICAgICBSZXZpZXdlZCBieSBD YW1lcm9uIFp3YXJpY2guCkluZGV4OiBKYXZhU2NyaXB0Q29yZS93dGYvVmVjdG9yLmgKPT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PQotLS0gSmF2YVNjcmlwdENvcmUvd3RmL1ZlY3Rvci5oCShyZXZpc2lvbiAzNjQwMCkKKysr IEphdmFTY3JpcHRDb3JlL3d0Zi9WZWN0b3IuaAkod29ya2luZyBjb3B5KQpAQCAtMzYsNiArMzYs NDQgQEAgbmFtZXNwYWNlIFdURiB7CiAgICAgdXNpbmcgc3RkOjptaW47CiAgICAgdXNpbmcgc3Rk OjptYXg7CiAgICAgCisgICAgLy8gV1RGX0FMSUdOX09GIC8gV1RGX0FMSUdORUQKKyAgICAjaWYg Q09NUElMRVIoR0NDKSB8fCBDT01QSUxFUihNSU5HVykKKyAgICAjZGVmaW5lIFdURl9BTElHTl9P Rih0eXBlKSAgIF9fYWxpZ25vZl9fKHR5cGUpCisgICAgI2RlZmluZSBXVEZfQUxJR05FRCh2YXJp YWJsZV90eXBlLCB2YXJpYWJsZSwgbikgdmFyaWFibGVfdHlwZSB2YXJpYWJsZSBfX2F0dHJpYnV0 ZV9fKChhbGlnbmVkKG4pKSkKKyAgICAjZWxpZiBDT01QSUxFUihNU1ZDKQorICAgICNkZWZpbmUg V1RGX0FMSUdOX09GKHR5cGUpICAgX19hbGlnbm9mKHR5cGUpCisgICAgI2RlZmluZSBXVEZfQUxJ R05FRCh2YXJpYWJsZV90eXBlLCB2YXJpYWJsZSwgbikgX19kZWNsc3BlYyhhbGlnbihuKSkgdmFy aWFibGVfdHlwZSB2YXJpYWJsZQorICAgICNlbHNlCisgICAgI2Vycm9yIG5lZWQgYWxpZ25tZW50 IGNvbnRyb2wKKyAgICAjZW5kaWYKKworICAgICNpZiBDT01QSUxFUihHQ0MpICYmICgoKF9fR05V Q19fICogMTAwKSArIF9fR05VQ19NSU5PUl9fKSA+PSAzMDMpCisgICAgICAgIHR5cGVkZWYgY2hh ciBfX2F0dHJpYnV0ZV9fKChfX21heV9hbGlhc19fKSkgQWxpZ25lZEJ1ZmZlckNoYXI7IAorICAg ICNlbHNlCisgICAgICAgIHR5cGVkZWYgY2hhciBBbGlnbmVkQnVmZmVyQ2hhcjsgCisgICAgI2Vu ZGlmCisKKyAgICB0ZW1wbGF0ZSA8c2l6ZV90IHNpemUsIHNpemVfdCBhbGlnbm1lbnQ+CisgICAg c3RydWN0IEFsaWduZWRCdWZmZXIgeyBBbGlnbmVkQnVmZmVyQ2hhciBidWZmZXJbc2l6ZV07IH07 CisKKyAgICB0ZW1wbGF0ZTxzaXplX3Qgc2l6ZT4KKyAgICBzdHJ1Y3QgQWxpZ25lZEJ1ZmZlcjxz aXplLCAyPiAgICB7IFdURl9BTElHTkVEKEFsaWduZWRCdWZmZXJDaGFyLCBidWZmZXJbc2l6ZV0s IDIpOyAgfTsKKworICAgIHRlbXBsYXRlPHNpemVfdCBzaXplPgorICAgIHN0cnVjdCBBbGlnbmVk QnVmZmVyPHNpemUsIDQ+ICAgIHsgV1RGX0FMSUdORUQoQWxpZ25lZEJ1ZmZlckNoYXIsIGJ1ZmZl cltzaXplXSwgNCk7ICB9OworCisgICAgdGVtcGxhdGU8c2l6ZV90IHNpemU+CisgICAgc3RydWN0 IEFsaWduZWRCdWZmZXI8c2l6ZSwgOD4gICAgeyBXVEZfQUxJR05FRChBbGlnbmVkQnVmZmVyQ2hh ciwgYnVmZmVyW3NpemVdLCA4KTsgIH07CisKKyAgICB0ZW1wbGF0ZTxzaXplX3Qgc2l6ZT4KKyAg ICBzdHJ1Y3QgQWxpZ25lZEJ1ZmZlcjxzaXplLCAxNj4gICB7IFdURl9BTElHTkVEKEFsaWduZWRC dWZmZXJDaGFyLCBidWZmZXJbc2l6ZV0sIDE2KTsgfTsKKworICAgIHRlbXBsYXRlPHNpemVfdCBz aXplPgorICAgIHN0cnVjdCBBbGlnbmVkQnVmZmVyPHNpemUsIDMyPiAgIHsgV1RGX0FMSUdORUQo QWxpZ25lZEJ1ZmZlckNoYXIsIGJ1ZmZlcltzaXplXSwgMzIpOyB9OworCisgICAgdGVtcGxhdGU8 c2l6ZV90IHNpemU+CisgICAgc3RydWN0IEFsaWduZWRCdWZmZXI8c2l6ZSwgNjQ+ICAgeyBXVEZf QUxJR05FRChBbGlnbmVkQnVmZmVyQ2hhciwgYnVmZmVyW3NpemVdLCA2NCk7IH07CisKICAgICB0 ZW1wbGF0ZSA8Ym9vbCBuZWVkc0Rlc3RydWN0aW9uLCB0eXBlbmFtZSBUPgogICAgIGNsYXNzIFZl Y3RvckRlc3RydWN0b3I7CiAKQEAgLTM4NCwxMCArNDIyLDkgQEAgbmFtZXNwYWNlIFdURiB7CiAg ICAgICAgIHVzaW5nIEJhc2U6Om1fY2FwYWNpdHk7CiAKICAgICAgICAgc3RhdGljIGNvbnN0IHNp emVfdCBtX2lubGluZUJ1ZmZlclNpemUgPSBpbmxpbmVDYXBhY2l0eSAqIHNpemVvZihUKTsKLSAg ICAgICAgVCogaW5saW5lQnVmZmVyKCkgeyByZXR1cm4gcmVpbnRlcnByZXRfY2FzdDxUKj4oJm1f aW5saW5lQnVmZmVyKTsgfQorICAgICAgICBUKiBpbmxpbmVCdWZmZXIoKSB7IHJldHVybiByZWlu dGVycHJldF9jYXN0PFQqPihtX2lubGluZUJ1ZmZlci5idWZmZXIpOyB9CiAKLSAgICAgICAgLy8g RklYTUU6IE5vdGhpbmcgZ3VhcmFudGVlcyB0aGlzIGJ1ZmZlciBpcyBhcHByb3ByaWF0ZWx5IGFs aWduZWQgdG8gaG9sZCBvYmplY3RzIG9mIHR5cGUgVC4KLSAgICAgICAgY2hhciBtX2lubGluZUJ1 ZmZlclttX2lubGluZUJ1ZmZlclNpemVdOworICAgICAgICBBbGlnbmVkQnVmZmVyPG1faW5saW5l QnVmZmVyU2l6ZSwgV1RGX0FMSUdOX09GKFQpPiBtX2lubGluZUJ1ZmZlcjsKICAgICB9OwogCiAg ICAgdGVtcGxhdGU8dHlwZW5hbWUgVCwgc2l6ZV90IGlubGluZUNhcGFjaXR5ID0gMD4K