Bug 202569 – [JSC] Change signature of HostFunction to (JSGlobalObject*, CallFrame*)

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Yusuke Suzuki]

...

[Radar WebKit Bug Importer]

<rdar://problem/55975858>

[Yusuke Suzuki]

Created attachment 380195 [details]
Patch

WIP

[Yusuke Suzuki]

Created attachment 380200 [details]
WIP

[Yusuke Suzuki]

Created attachment 380207 [details]
WIP

[Yusuke Suzuki]

OK, let's see the status of EWS.

[Yusuke Suzuki]

Created attachment 380237 [details]
Patch

[Yusuke Suzuki]

Created attachment 380247 [details]
Patch

[Yusuke Suzuki]

Created attachment 380260 [details]
Patch

[Yusuke Suzuki]

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.

[Saam Barati]

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)

[Yusuke Suzuki]

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

[Yusuke Suzuki]

Committed r250803: <https://trac.webkit.org/changeset/250803>

[Yusuke Suzuki]

Committed r250807: <https://trac.webkit.org/changeset/250807>

[Paulo Matos]

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

[Truitt Savell]

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

[Yusuke Suzuki]

Committed r250982: <https://trac.webkit.org/changeset/250982>