[webkit-reviews] review denied: [Bug 16925] How to fix VectorBuffer's alignment FIXME : [Attachment 23467] Patch which resolves this problem.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Sep 16 09:51:16 PDT 2008


Darin Adler <darin at apple.com> has denied Paul Pedriana <ppedriana at gmail.com>'s
request for review:
Bug 16925: How to fix VectorBuffer's alignment FIXME
https://bugs.webkit.org/show_bug.cgi?id=16925

Attachment 23467: Patch which resolves this problem.
https://bugs.webkit.org/attachment.cgi?id=23467&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
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.


More information about the webkit-reviews mailing list