Bug 202569 - [JSC] Change signature of HostFunction to (JSGlobalObject*, CallFrame*)
Summary: [JSC] Change signature of HostFunction to (JSGlobalObject*, CallFrame*)
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords: InRadar
Depends on:
Blocks: 202392
  Show dependency treegraph
 
Reported: 2019-10-04 00:46 PDT by Yusuke Suzuki
Modified: 2019-10-10 12:40 PDT (History)
33 users (show)

See Also:


Attachments
Patch (1.21 MB, patch)
2019-10-04 01:30 PDT, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
WIP (1.23 MB, patch)
2019-10-04 02:04 PDT, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
WIP (1.24 MB, patch)
2019-10-04 02:56 PDT, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (1.24 MB, patch)
2019-10-04 13:01 PDT, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (1.24 MB, patch)
2019-10-04 13:37 PDT, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (1.24 MB, patch)
2019-10-04 15:39 PDT, Yusuke Suzuki
sbarati: 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 2019-10-04 00:46:13 PDT
...
Comment 1 Radar WebKit Bug Importer 2019-10-04 00:47:00 PDT
<rdar://problem/55975858>
Comment 2 Yusuke Suzuki 2019-10-04 01:30:49 PDT
Created attachment 380195 [details]
Patch

WIP
Comment 3 Yusuke Suzuki 2019-10-04 02:04:10 PDT
Created attachment 380200 [details]
WIP
Comment 4 Yusuke Suzuki 2019-10-04 02:56:36 PDT
Created attachment 380207 [details]
WIP
Comment 5 Yusuke Suzuki 2019-10-04 02:59:35 PDT
OK, let's see the status of EWS.
Comment 6 Yusuke Suzuki 2019-10-04 13:01:54 PDT
Created attachment 380237 [details]
Patch
Comment 7 Yusuke Suzuki 2019-10-04 13:37:18 PDT
Created attachment 380247 [details]
Patch
Comment 8 Yusuke Suzuki 2019-10-04 15:39:07 PDT
Created attachment 380260 [details]
Patch
Comment 9 Yusuke Suzuki 2019-10-04 15:41:39 PDT
Comment on attachment 380260 [details]
Patch

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

The other parts are mechanical one.

> Source/JavaScriptCore/interpreter/Interpreter.cpp:840
>      ProtoCallFrame protoCallFrame;

ProtoCallFrame's globalObject change is important.

> Source/JavaScriptCore/jit/ThunkGenerators.cpp:273
>      // Calling convention:      f(edi, esi, edx, ecx, ...);

ThunkGenerators.cpp change is important.

> Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm:265
> +    makeCall(entry, protoCallFrame, t3, t4)

LLInt change is important.
Comment 10 Saam Barati 2019-10-07 15:30:43 PDT
Comment on attachment 380260 [details]
Patch

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

r=me

>> Source/JavaScriptCore/jit/ThunkGenerators.cpp:273
>>      // Calling convention:      f(edi, esi, edx, ecx, ...);
> 
> ThunkGenerators.cpp change is important.

can we file a bug on using GPRInfo here instead of naming registers?

> Source/JavaScriptCore/jit/ThunkGenerators.cpp:337
> +    static_assert(GPRInfo::regT2 != GPRInfo::argumentGPR0);
> +    static_assert(GPRInfo::regT2 != GPRInfo::argumentGPR1);

I think arm64 and x86_64 on Windows/*nix can easily be written using GPRInfo and RegisterSet. As I said above, filing a bug for this work would be nice (you don't necessarily need to do the work, but we should just have a bug)
Comment 11 Yusuke Suzuki 2019-10-07 15:47:46 PDT
Comment on attachment 380260 [details]
Patch

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

Thanks!

>>> Source/JavaScriptCore/jit/ThunkGenerators.cpp:273
>>>      // Calling convention:      f(edi, esi, edx, ecx, ...);
>> 
>> ThunkGenerators.cpp change is important.
> 
> can we file a bug on using GPRInfo here instead of naming registers?

Yes, filed https://bugs.webkit.org/show_bug.cgi?id=202657.

>> Source/JavaScriptCore/jit/ThunkGenerators.cpp:337
>> +    static_assert(GPRInfo::regT2 != GPRInfo::argumentGPR1);
> 
> I think arm64 and x86_64 on Windows/*nix can easily be written using GPRInfo and RegisterSet. As I said above, filing a bug for this work would be nice (you don't necessarily need to do the work, but we should just have a bug)

Right. Fixed. https://bugs.webkit.org/show_bug.cgi?id=202657
Comment 12 Yusuke Suzuki 2019-10-07 16:13:52 PDT
Committed r250803: <https://trac.webkit.org/changeset/250803>
Comment 13 Yusuke Suzuki 2019-10-07 16:53:34 PDT
Committed r250807: <https://trac.webkit.org/changeset/250807>
Comment 14 Paulo Matos 2019-10-09 06:32:53 PDT
Since this bug's patch made trunk, that we have 32bits failing to build in Debug mode or segfaulting in tests when built in Release mode - opened bug 202748
Comment 15 Truitt Savell 2019-10-09 10:58:41 PDT
It looks like the changes in r250803 and the subsequent change in 250807 has caused the windows builds to fail. 

build:
https://build.webkit.org/builders/Apple%20Win%2010%20Debug%20%28Build%29/builds/7170
https://build.webkit.org/builders/Apple%20Win%2010%20Debug%20%28Build%29/builds/7173
Comment 16 Yusuke Suzuki 2019-10-10 12:40:54 PDT
Committed r250982: <https://trac.webkit.org/changeset/250982>