InById operation is now ad-hocly optimized. We should align it to GetById etc. This bug introduces op_in_by_id, and op_in_by_val. And DFG should have InById and InByVal. In DFG AI, and other places we are attempting to convert InByVal to InById. So current ad-hoc In IC can be refactored.
(In reply to Yusuke Suzuki from comment #0) > InById operation is now ad-hocly optimized. We should align it to GetById > etc. > This bug introduces op_in_by_id, and op_in_by_val. And DFG should have > InById and InByVal. > In DFG AI, and other places we are attempting to convert InByVal to InById. > So current ad-hoc In IC can be refactored. And we have a chance to fold it to constant :)
Created attachment 340758 [details] WIP
Attachment 340758 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/jit/JITInlineCacheGenerator.cpp:172: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4] ERROR: Source/JavaScriptCore/jit/JITInlineCacheGenerator.cpp:173: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4] ERROR: Source/JavaScriptCore/jit/JIT.cpp:275: Non-label code inside switch statements should be indented. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/jit/JITInlineCacheGenerator.h:152: The parameter name "value" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 4 in 53 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 340773 [details] Patch
Attachment 340773 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/jit/JITInlineCacheGenerator.cpp:172: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4] ERROR: Source/JavaScriptCore/jit/JITInlineCacheGenerator.cpp:173: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4] ERROR: Source/JavaScriptCore/jit/JIT.cpp:275: Non-label code inside switch statements should be indented. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/jit/JITInlineCacheGenerator.h:152: The parameter name "value" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 4 in 48 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 340775 [details] Patch
Attachment 340775 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/jit/JITInlineCacheGenerator.cpp:172: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4] ERROR: Source/JavaScriptCore/jit/JITInlineCacheGenerator.cpp:173: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4] ERROR: Source/JavaScriptCore/jit/JIT.cpp:275: Non-label code inside switch statements should be indented. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/jit/JITInlineCacheGenerator.h:152: The parameter name "value" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 4 in 48 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 340777 [details] Patch
Attachment 340777 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/jit/JITInlineCacheGenerator.cpp:172: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4] ERROR: Source/JavaScriptCore/jit/JIT.cpp:275: Non-label code inside switch statements should be indented. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/jit/JITInlineCacheGenerator.h:152: The parameter name "value" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 3 in 56 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 340779 [details] Patch
Attachment 340779 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/jit/JITInlineCacheGenerator.cpp:172: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4] ERROR: Source/JavaScriptCore/jit/JIT.cpp:275: Non-label code inside switch statements should be indented. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/jit/JITInlineCacheGenerator.h:152: The parameter name "value" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 3 in 56 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 340783 [details] Patch
Attachment 340783 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/jit/JITInlineCacheGenerator.cpp:172: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4] ERROR: Source/JavaScriptCore/jit/JIT.cpp:275: Non-label code inside switch statements should be indented. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/jit/JITInlineCacheGenerator.h:152: The parameter name "value" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 3 in 56 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 340783 [details] Patch I love this change. I was wanting to do this the whole time I was writing the instanceof IC.
Committed r231998: <https://trac.webkit.org/changeset/231998>
<rdar://problem/40395093>
Committed r232017: <https://trac.webkit.org/changeset/232017>
(In reply to Yusuke Suzuki from comment #17) > Committed r232017: <https://trac.webkit.org/changeset/232017> 32-bit JSC tests are still failing assertions after this fix: https://build.webkit.org/builders/Apple%20High%20Sierra%2032-bit%20JSC%20%28BuildAndTest%29/builds/1957
Re-opened since this is blocked by bug 185842
I've fixed this patch to also work on 32-bit architectures in this patch: https://bugs.webkit.org/show_bug.cgi?id=185845.
Comment on attachment 340783 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=340783&action=review > Source/JavaScriptCore/jit/JITInlineCacheGenerator.cpp:181 > + generateFastCommon(jit, InlineAccess::sizeForPropertyAccess()); Don't we want a size tuned for "in"?
Committed r232047: <https://trac.webkit.org/changeset/232047>
Comment on attachment 340783 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=340783&action=review >> Source/JavaScriptCore/jit/JITInlineCacheGenerator.cpp:181 >> + generateFastCommon(jit, InlineAccess::sizeForPropertyAccess()); > > Don't we want a size tuned for "in"? I think this is enough for now, since the changes between load v.s. in is very trivial (load property v.s. move constant).
Sweet! I think it is a good time to get back work into https://bugs.webkit.org/show_bug.cgi?id=172888