Summary: | Opcode.h use const void* for Opcode cause error #1211 for RVCT compiler | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Lyon Chen <lyon.chen> | ||||||||
Component: | JavaScriptCore | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | commit-queue, ggaren, staikos, yong.li.webkit | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | All | ||||||||||
OS: | All | ||||||||||
Attachments: |
|
Description
Lyon Chen
2010-01-20 07:24:02 PST
Created attachment 47035 [details] Patch for bug 33902 Comment on attachment 47035 [details] Patch for bug 33902 Two problems here: 1) We don't accept patches without change log entries. 2) This is probably not something that should be ifdef'd based on compiler. If void* works for the other compilers, then the patch should just switch from const void* to void*. Darin: In commit r52231, ggaren@apple.com mentioned the change from void* to const void* is to avoid generating bloated codes in interpreter.cpp, so maybe it's better #ifdef this change? And maybe RVCT compiler can someday updated to support const void* for computed goto, instead of void*? Hope to hear from ggaren@apple.com on this matter. > In commit r52231, ggaren@apple.com mentioned the change from void* to const
> void* is to avoid generating bloated codes in interpreter.cpp, so maybe it's
> better #ifdef this change?
I'm not sure if that specific part of the change is required. I guess you'd have to test to find out.
Created attachment 47128 [details]
New patch with changelog.
Did you verify that this change doesn't change code generation for non-RVCT compilers? No, I didn't, Garen. I thought your comment means this part is not essential to your optimization, and based on Darin's comments, I simply removed the #ifdef for RVCT. Sorry I don't have the time to verify it for other platforms for now, should I add these #ifdef back? +George Staikos Created attachment 47131 [details]
New patch with RVCT flag.
Add a new patch with code change limited to RVCT compiler.
Comment on attachment 47128 [details]
New patch with changelog.
I’ll let Geoff handle reviewing duties on this bug. I’m not sure I know what the issues are and I know he does.
Comment on attachment 47131 [details]
New patch with RVCT flag.
Seems fine to have this scoped to COMPILER(RCVT)
Comment on attachment 47131 [details] New patch with RVCT flag. Rejecting patch 47131 from commit-queue. lyon.chen@torchmobile.com does not have committer permissions according to http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/webkitpy/committers.py. - If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags. - If you have committer rights please correct the error in WebKitTools/Scripts/webkitpy/committers.py by adding yourself to the file (no review needed). Due to bug 30084 the commit-queue will require a restart after your change. Please contact eseidel@chromium.org to request a commit-queue restart. After restart the commit-queue will correctly respect your committer rights. Comment on attachment 47131 [details] New patch with RVCT flag. Clearing flags on attachment: 47131 Committed r53891: <http://trac.webkit.org/changeset/53891> All reviewed patches have been landed. Closing bug. |