Summary: | [JSC] JSC should have consistent InById IC | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Yusuke Suzuki <ysuzuki> | ||||||||||||||
Component: | JavaScriptCore | Assignee: | Yusuke Suzuki <ysuzuki> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | commit-queue, dominik.infuehr, ews-watchlist, fpizlo, keith_miller, mark.lam, msaboff, ryanhaddad, saam, ticaiolima, webkit-bug-importer | ||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||
OS: | Unspecified | ||||||||||||||||
Bug Depends on: | 185842 | ||||||||||||||||
Bug Blocks: | 185803 | ||||||||||||||||
Attachments: |
|
Description
Yusuke Suzuki
2018-05-16 09:51:33 PDT
(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> 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 |