Summary: | [JSC] Use FixedVector more in bytecode dir and JumpTable | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Yusuke Suzuki <ysuzuki> | ||||
Component: | New Bugs | Assignee: | Yusuke Suzuki <ysuzuki> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | benjamin, cdumez, cmarcelo, ews-watchlist, keith_miller, mark.lam, msaboff, saam, tzagallo, webkit-bug-importer | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | WebKit Nightly Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Yusuke Suzuki
2021-04-07 02:37:44 PDT
Created attachment 425372 [details]
Patch
Comment on attachment 425372 [details]
Patch
r=me
Comment on attachment 425372 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=425372&action=review r=me with issues addressed. > Source/JavaScriptCore/ChangeLog:8 > + 1. Use FixedVector more in bytecode/ directory's long-living data structures.[ Accidental '['. > Source/JavaScriptCore/ChangeLog:9 > + 2. Use FixedVector in SimpleJumpTable. This involves LLInt changes because we need to access to FixedVector data from LLInt. /access to/access/ > Source/JavaScriptCore/bytecode/ObjectPropertyConditionSet.h:61 > + result.m_data->m_vector = FixedVector<ObjectPropertyCondition>(WTFMove(vector)); Shouldn't this be: result.m_data->m_vector = WTFMove(FixedVector<ObjectPropertyCondition>(vector)); > Source/JavaScriptCore/llint/LowLevelInterpreter.asm:616 > +const RefCountedArrayStorageNonNullRefCountOffset = -(constexpr (RefCountedArray::Header::size())) + RefCountedArray::Header::refCount Is this needed? > Source/WTF/wtf/RefCountedArray.h:280 > + static ptrdiff_t offsetOfRefCount() { return OBJECT_OFFSETOF(Header, refCount); } > + static ptrdiff_t offsetOfLength() { return OBJECT_OFFSETOF(Header, length); } Are these used anywhere? Comment on attachment 425372 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=425372&action=review >> Source/JavaScriptCore/bytecode/ObjectPropertyConditionSet.h:61 >> + result.m_data->m_vector = FixedVector<ObjectPropertyCondition>(WTFMove(vector)); > > Shouldn't this be: > result.m_data->m_vector = WTFMove(FixedVector<ObjectPropertyCondition>(vector)); This becomes move-assignment without WTFMove. Comment on attachment 425372 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=425372&action=review >> Source/JavaScriptCore/ChangeLog:8 >> + 1. Use FixedVector more in bytecode/ directory's long-living data structures.[ > > Accidental '['. Fixed. >> Source/JavaScriptCore/ChangeLog:9 >> + 2. Use FixedVector in SimpleJumpTable. This involves LLInt changes because we need to access to FixedVector data from LLInt. > > /access to/access/ Fixed. >> Source/JavaScriptCore/llint/LowLevelInterpreter.asm:616 >> +const RefCountedArrayStorageNonNullRefCountOffset = -(constexpr (RefCountedArray::Header::size())) + RefCountedArray::Header::refCount > > Is this needed? I added for the completeness, but maybe we never access it. Removed. >> Source/WTF/wtf/RefCountedArray.h:280 >> + static ptrdiff_t offsetOfLength() { return OBJECT_OFFSETOF(Header, length); } > > Are these used anywhere? Not used. But I think having offsetOfLength() is good since we could touch it from JIT in the future, and we know that this field is accessed by LLInt by having this function too. Committed r275626 (236271@main): <https://commits.webkit.org/236271@main> |