Bug 162978 - [DOMJIT] Support slow path call
Summary: [DOMJIT] Support slow path call
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:
Depends on: 162941
Blocks: 162544 163005
  Show dependency treegraph
 
Reported: 2016-10-05 10:39 PDT by Yusuke Suzuki
Modified: 2016-10-06 22:09 PDT (History)
3 users (show)

See Also:


Attachments
Patch (38.12 KB, patch)
2016-10-06 02:51 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 2016-10-05 10:39:00 PDT
DOMJIT::Patchpoint (Maybe, DOMJIT::PatchpointParams) should have ability to request slow path call in both DFG and FTL!
Comment 1 Yusuke Suzuki 2016-10-05 10:49:10 PDT
If we expose variadic template-ed slow path generator that calls DFG / FTL slow path generators, it exposes almost all the DFG and FTL.
For example, I tested the above design. And I ends up with exposing DFGSpeculativeJIT! That's bad.

Instead, I'll take callOperation like design. We list up necessary calls in DFGSlowPathCall.h. And it will invoke DFG / FTL's appropriate functionality in DFGSlowPathCall.cpp.
Comment 2 Yusuke Suzuki 2016-10-06 02:51:05 PDT
Created attachment 290802 [details]
Patch
Comment 3 Saam Barati 2016-10-06 19:15:53 PDT
Comment on attachment 290802 [details]
Patch

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

> Source/JavaScriptCore/domjit/DOMJITSlowPathCalls.h:32
> +#define DOMJIT_SLOW_PATH_CALLS(macro) \
> +    macro(J_JITOperation_EP, JSValueRegs, GPRReg) \

I wonder if it ever makes sense to expose a call API that's more abstract like this:

class Call {
      struct Argument {
              Argument(GPRReg);
              Argument(JSValueRegs);
              Argument(immediate type: int32/int64/pointer); 
      }
      Type m_resultType;
      Vector<Arguments> m_arguments;
      FunctionPtr m_function;
};
and then have a function like:
setupCallWithExecState(const Call& call);

> Source/JavaScriptCore/ftl/FTLDOMJITPatchpointParams.cpp:41
> +    params.addLatePath([=] (CCallHelpers& jit) {

I wonder if it ever makes sense to have a normal call API.
Comment 4 Yusuke Suzuki 2016-10-06 21:37:38 PDT
Comment on attachment 290802 [details]
Patch

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

>> Source/JavaScriptCore/ftl/FTLDOMJITPatchpointParams.cpp:41
>> +    params.addLatePath([=] (CCallHelpers& jit) {
> 
> I wonder if it ever makes sense to have a normal call API.

Oops, thanks. It's OK. fixed.
Comment 5 Yusuke Suzuki 2016-10-06 21:38:13 PDT
Comment on attachment 290802 [details]
Patch

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

>> Source/JavaScriptCore/domjit/DOMJITSlowPathCalls.h:32
>> +    macro(J_JITOperation_EP, JSValueRegs, GPRReg) \
> 
> I wonder if it ever makes sense to expose a call API that's more abstract like this:
> 
> class Call {
>       struct Argument {
>               Argument(GPRReg);
>               Argument(JSValueRegs);
>               Argument(immediate type: int32/int64/pointer); 
>       }
>       Type m_resultType;
>       Vector<Arguments> m_arguments;
>       FunctionPtr m_function;
> };
> and then have a function like:
> setupCallWithExecState(const Call& call);

Looks clean. I filed it in https://bugs.webkit.org/show_bug.cgi?id=163099
Comment 6 Yusuke Suzuki 2016-10-06 21:50:12 PDT
Comment on attachment 290802 [details]
Patch

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

>>> Source/JavaScriptCore/ftl/FTLDOMJITPatchpointParams.cpp:41
>>> +    params.addLatePath([=] (CCallHelpers& jit) {
>> 
>> I wonder if it ever makes sense to have a normal call API.
> 
> Oops, thanks. It's OK. fixed.

But, after considering, the above one is still useful. We can emit slow path after the fast path sequence by using addLatePath.
Comment 7 Yusuke Suzuki 2016-10-06 22:09:45 PDT
Committed r206899: <http://trac.webkit.org/changeset/206899>