In commit r52231, Opcode is changed from void* to const void* when COMPUTED_GOTO is used, this caused a compiling error #1211 for RVCT 4.0 compilers. Suggest to change to: --- a/JavaScriptCore/bytecode/Opcode.h +++ b/JavaScriptCore/bytecode/Opcode.h @@ -196,7 +196,11 @@ #undef VERIFY_OPCODE_ID #if HAVE(COMPUTED_GOTO) +#if COMPILER(RVCT) + typedef void* Opcode; +#else typedef const void* Opcode; +#endif #else typedef OpcodeID Opcode; #endif
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.