Bug 180804 – [DFG] Reduce register pressure of WeakMapGet to be used for 32bit

• Component: New Bugs
• Status: RESOLVED FIXED

[Yusuke Suzuki]

[DFG] Reduce register pressure of WeakMapGet to be used for 32bit

[Yusuke Suzuki]

Created attachment 329343 [details]
Patch

[Yusuke Suzuki]

Comment on attachment 329343 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=329343&action=review

> Source/JavaScriptCore/dfg/DFGRegisterBank.h:150
> +    }

Helper method for debugging.

[Saam Barati]

Comment on attachment 329343 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=329343&action=review

r=me

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:10977
> +    std::optional<GPRTemporary> index;

Why optional? Does GPRTemporary have an empty constructor? If not, maybe it’s worth adding and you can just WTFMove() to it below instead of emplace

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:10982
> +        index.emplace(this, Reuse, hash);

Does Reuse only reuse if we’re the last user of the value?

[Yusuke Suzuki]

Comment on attachment 329343 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=329343&action=review

>> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:10977
>> +    std::optional<GPRTemporary> index;
> 
> Why optional? Does GPRTemporary have an empty constructor? If not, maybe it’s worth adding and you can just WTFMove() to it below instead of emplace

OK, changed to using WTFMove(GPRTemporary()).

>> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:10982
>> +        index.emplace(this, Reuse, hash);
> 
> Does Reuse only reuse if we’re the last user of the value?

Yes, so this Reuse is just an optimization.

[Yusuke Suzuki]

Committed r225952: <https://trac.webkit.org/changeset/225952>

[Radar WebKit Bug Importer]

<rdar://problem/36064081>

[Yusuke Suzuki]

Committed r225971: <https://trac.webkit.org/changeset/225971>