r65596 adjusted ENABLE_PROFILER_REFERENCE_OFFSET so that it was no longer divisible by 8. ENABLE_PROFILER_REFERENCE_OFFSET is used to adjust the stack pointer in the cti methods and 65596 will cause the stack pointer to no longer be 8 byte aligned. According to the ABI 8 byte stack alignment should be preserved.
Created attachment 67411 [details] Ensure ENABLE_PROFILER_REFERENCE_OFFSET is divisible by 8 This change adjusts the padding in the struct. In r65596 the padding that was there for JSVALUE32_64 was made useless because the object would be 8 byte aligned already so it can be removed. The additional padding that was essentially used for making sure the enabledProfilerReference offset was 8 byte aligned for JSVALUE32 was moved ahead so that we remove this padding for JSVALUE32_64 and have the correct field alignments for the rest. One benefit of this change (aside from fixing the bug) is that it reduces the JITStackFrame by 12 bytes by removing useless padding.
Attachment 67411 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 JavaScriptCore/jit/JITStubs.h:151: One space before end of line comments [whitespace/comments] [5] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 67414 [details] Fix style of previous patch - Ensure ENABLE_PROFILER_REFERENCE_OFFSET is divisible by 8
Looks fine, but please allow me to test this patch on Darwin before landing. Will do so tomorrow, and review then.
Works fine here. Thanks for the fix!
(In reply to comment #5) > Works fine here. Thanks for the fix! So who can give me a r+ and a commit-queue+?
> So who can give me a r+ and a commit-queue+? Please wait for Gavin to test your patch on Darwin!
The commit-queue will "test" (run the layout tests and javascriptcore tests) on Darwin of course. :) Depends on what you need to test.
Comment on attachment 67414 [details] Fix style of previous patch - Ensure ENABLE_PROFILER_REFERENCE_OFFSET is divisible by 8 Sorry for holding this up. My main dev machine has melted itself in a pretty catastrophic way today & will be unable to test for at least another day. I don't think it would be reasonable for me to delay you any further so I'll get out of the way & r+. If this does raise any issues we can sort this out afterwards.
(In reply to comment #9) > (From update of attachment 67414 [details]) > Sorry for holding this up. My main dev machine has melted itself in a pretty catastrophic way today & will be unable to test for at least another day. I don't think it would be reasonable for me to delay you any further so I'll get out of the way & r+. > If this does raise any issues we can sort this out afterwards. I don't think I attached a changelog so it should probably be r- until I do that.
Comment on attachment 67414 [details] Fix style of previous patch - Ensure ENABLE_PROFILER_REFERENCE_OFFSET is divisible by 8 Ooops, good point. :-)
Created attachment 68108 [details] Same patch as 67414 but with prepare-ChangeLog run
Comment on attachment 68108 [details] Same patch as 67414 but with prepare-ChangeLog run This patch already contains the previously missing changelog, and doesn't make any other changes.
Comment on attachment 68108 [details] Same patch as 67414 but with prepare-ChangeLog run Clearing flags on attachment: 68108 Committed r67943: <http://trac.webkit.org/changeset/67943>
All reviewed patches have been landed. Closing bug.