Bug 184316 - Add pointer profiling support to the DFG and supporting files.
Summary: Add pointer profiling support to the DFG and supporting files.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Lam
URL:
Keywords: InRadar
Depends on:
Blocks: 184324
  Show dependency treegraph
 
Reported: 2018-04-04 14:38 PDT by Mark Lam
Modified: 2018-04-04 21:31 PDT (History)
8 users (show)

See Also:


Attachments
proposed patch. (91.78 KB, patch)
2018-04-04 17:17 PDT, Mark Lam
fpizlo: review+
Details | Formatted Diff | Diff
patch for landing. (91.87 KB, patch)
2018-04-04 20:07 PDT, Mark Lam
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Lam 2018-04-04 14:38:19 PDT
Patch coming.
Comment 1 Radar WebKit Bug Importer 2018-04-04 14:39:03 PDT
<rdar://problem/39188524>
Comment 2 Mark Lam 2018-04-04 17:17:10 PDT
Created attachment 337245 [details]
proposed patch.

Let's get some baking time on the EWS.
Comment 3 EWS Watchlist 2018-04-04 17:19:40 PDT
Attachment 337245 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/dfg/DFGSlowPathGenerator.h:170:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
Total errors found: 1 in 30 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Filip Pizlo 2018-04-04 17:37:22 PDT
Comment on attachment 337245 [details]
proposed patch.

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

> Source/JavaScriptCore/b3/air/AirCCallSpecial.cpp:137
> +        jit.call(scratchRegister, B3CCallPtrTag);

This might be OK, but can you file a bug for passing the ptrtag to the CCallValue, so that the client of B3 can select what tag is used for each call they emit?

> Source/JavaScriptCore/dfg/DFGOSRExit.cpp:755
> +#if USE(POINTER_PROFILING)
> +            void* oldEntrySP = cpu.fp<uint8_t*>() + sizeof(CallerFrameAndPC);
> +            void* newEntrySP = cpu.fp<uint8_t*>() + inlineCallFrame->returnPCOffset() + sizeof(void*);
> +            returnPC = retagCodePtr(returnPC, bitwise_cast<PtrTag>(oldEntrySP), bitwise_cast<PtrTag>(newEntrySP));
> +#endif

Wait... won't this fail miserably if the OSR exit is reused at a different stack height?
Comment 5 Filip Pizlo 2018-04-04 17:43:45 PDT
Comment on attachment 337245 [details]
proposed patch.

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

OK, now that Mark explained this to me, I get it.  r=me

>> Source/JavaScriptCore/dfg/DFGOSRExit.cpp:755
>> +#endif
> 
> Wait... won't this fail miserably if the OSR exit is reused at a different stack height?

Never mind, this is the probe code.  It's right.
Comment 6 Mark Lam 2018-04-04 20:07:22 PDT
Created attachment 337254 [details]
patch for landing.
Comment 7 EWS Watchlist 2018-04-04 20:10:15 PDT
Attachment 337254 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/dfg/DFGSlowPathGenerator.h:170:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
Total errors found: 1 in 30 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 8 Mark Lam 2018-04-04 21:31:03 PDT
Thanks for the review.  Landed in r230294: <http://trac.webkit.org/r230294>.