Bug 224275 - [JSC] Use FixedVector more in bytecode dir and JumpTable
Summary: [JSC] Use FixedVector more in bytecode dir and JumpTable
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-04-07 02:37 PDT by Yusuke Suzuki
Modified: 2021-04-07 14:16 PDT (History)
10 users (show)

See Also:


Attachments
Patch (30.45 KB, patch)
2021-04-07 02:41 PDT, Yusuke Suzuki
msaboff: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yusuke Suzuki 2021-04-07 02:37:44 PDT
[JSC] Use FixedVector more in bytecode dir and JumpTable
Comment 1 Yusuke Suzuki 2021-04-07 02:41:53 PDT
Created attachment 425372 [details]
Patch
Comment 2 Michael Saboff 2021-04-07 11:39:33 PDT
Comment on attachment 425372 [details]
Patch

r=me
Comment 3 Mark Lam 2021-04-07 11:53:33 PDT
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 4 Yusuke Suzuki 2021-04-07 13:13:49 PDT
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 5 Yusuke Suzuki 2021-04-07 13:15:05 PDT
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.
Comment 6 Yusuke Suzuki 2021-04-07 14:15:10 PDT
Committed r275626 (236271@main): <https://commits.webkit.org/236271@main>
Comment 7 Radar WebKit Bug Importer 2021-04-07 14:16:16 PDT
<rdar://problem/76365012>