The old JIT optimizes intrinsics with ThunkGenerators, which are functions that when called generate JIT code that implements the intrinsic. The DFG JIT optimizes intrinsics by weaving them through the optimizing compiler, and only exposes an enum (DFG::Intrinsic) for declaring whether or not an ExecutableBase is intrinsic, and if so, which intrinsic it is. These two mechanism require some redundancy in runtime/Lookup.h, and various scripts for creating JS bindings. A good solution would be to have one enum (say, in jit/JITIntrinsic.h). The DFG would simply switch to using this intrinsic, while the old JIT would abandon passing function pointers and simply select the thunk generator by switching on the intrinsic enum.
If you are going to make this change, it might also be a good opportunity to remove the special casing from the generation script and put it in the lookup definitions.
It may even be that we don't want the enum to be in either JIT - we may with to make use of the intrinsic from the interpreter, in environments where we don't have the JIT enabled. Maybe an enum in JSFunction?
(In reply to comment #2) > It may even be that we don't want the enum to be in either JIT - we may with to make use of the intrinsic from the interpreter, in environments where we don't have the JIT enabled. Maybe an enum in JSFunction? How about runtime/Intrinsic.h? The benefit of a separate file is that: 1) JSFunction and pretty much everything else in runtime/ is cluttered. 2) If we do our jobs correctly then this enum will get big. ;-)
Agreed, that sounds even better.
Created attachment 117999 [details] Fix
Comment on attachment 117999 [details] Fix r=me
Comment on attachment 117999 [details] Fix Attachment 117999 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/10690698
Comment on attachment 117999 [details] Fix Attachment 117999 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/10728902
Comment on attachment 117999 [details] Fix Attachment 117999 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/10729969
Comment on attachment 117999 [details] Fix View in context: https://bugs.webkit.org/attachment.cgi?id=117999&action=review > Source/JavaScriptCore/create_hash_table:276 > if ($key eq "charCodeAt") { > $thunkGenerator = "charCodeAtThunkGenerator"; > - $intrinsic = "DFG::CharCodeAtIntrinsic"; > + $intrinsic = "CharCodeAtIntrinsic"; Do we really need this specialized check? Can't we just write CharCodeAtIntrinsic at the definition site? > Source/JavaScriptCore/create_hash_table:323 > $thunkGenerator = "logThunkGenerator"; > + $intrinsic = "LogIntrinsic"; Is $thunkGenerator still being used?
(In reply to comment #10) > (From update of attachment 117999 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=117999&action=review > > > Source/JavaScriptCore/create_hash_table:276 > > if ($key eq "charCodeAt") { > > $thunkGenerator = "charCodeAtThunkGenerator"; > > - $intrinsic = "DFG::CharCodeAtIntrinsic"; > > + $intrinsic = "CharCodeAtIntrinsic"; > > Do we really need this specialized check? Can't we just write CharCodeAtIntrinsic at the definition site? I definitely agree with you that we should do this, but I don't think it's a part of this change. This patch changes the data in the table output by the create_hash_table script, you're proposing changing the data in the tables parsed in by create_hash_table, these two changes are completely independent of each other. > > Source/JavaScriptCore/create_hash_table:323 > > $thunkGenerator = "logThunkGenerator"; > > + $intrinsic = "LogIntrinsic"; > > Is $thunkGenerator still being used? Ooops - meant to remove them - thanks! - will do.
Fixed in r102167