Bug 178949

Summary:

Bytecode liveness should live on UnlinkedCodeBlock so it can be shared amongst CodeBlocks

Product:

WebKit

Reporter:

Saam Barati <saam>

Component:

JavaScriptCore

Assignee:

Saam Barati <saam>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

benjamin, buildbot, commit-queue, fpizlo, ggaren, gskachkov, jfbastien, keith_miller, mark.lam, msaboff, rmorisset, ticaiolima, webkit-bug-importer, ysuzuki

Priority:

Keywords:

InRadar

Version:

Safari Technology Preview

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
WIP	none
WIP	none
patch	none
patch	none
patch	none
patch	none

Saam Barati

Reported 2017-10-27 12:57:42 PDT

...

Attachments
WIP (33.44 KB, patch) 2017-10-27 14:52 PDT, Saam Barati	no flags	Details Formatted Diff Diff
WIP (36.68 KB, patch) 2017-10-27 15:12 PDT, Saam Barati	no flags	Details Formatted Diff Diff
patch (41.21 KB, patch) 2017-10-27 16:00 PDT, Saam Barati	no flags	Details Formatted Diff Diff
patch (41.26 KB, patch) 2017-10-27 16:09 PDT, Saam Barati	no flags	Details Formatted Diff Diff
patch (41.44 KB, patch) 2017-10-27 16:16 PDT, Saam Barati	no flags	Details Formatted Diff Diff
patch (41.17 KB, patch) 2017-10-27 16:55 PDT, Saam Barati	no flags	Details Formatted Diff Diff
Show Obsolete (5) View All Add attachment proposed patch, testcase, etc.

Saam Barati

Comment 1 2017-10-27 14:52:45 PDT

Created attachment 325201 [details] WIP refactoring

Saam Barati

Comment 2 2017-10-27 15:12:35 PDT

Created attachment 325204 [details] WIP

Saam Barati

Comment 3 2017-10-27 15:14:42 PDT

(In reply to Saam Barati from comment #2) > Created attachment 325204 [details] > WIP It seems to work.

Build Bot

Comment 4 2017-10-27 15:15:11 PDT

Attachment 325204 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/bytecode/BytecodeLivenessAnalysis.cpp:89: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/bytecode/BytecodeLivenessAnalysis.cpp:91: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/bytecode/BytecodeLivenessAnalysis.cpp:92: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/bytecode/BytecodeLivenessAnalysis.cpp:93: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/bytecode/BytecodeLivenessAnalysis.cpp:95: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/bytecode/BytecodeLivenessAnalysis.cpp:117: Should have a space between // and comment [whitespace/comments] [4] Total errors found: 6 in 14 files If any of these errors are false positives, please file a bug against check-webkit-style.

Saam Barati

Comment 5 2017-10-27 16:00:41 PDT

Created attachment 325212 [details] patch

Saam Barati

Comment 6 2017-10-27 16:09:04 PDT

Created attachment 325213 [details] patch

Saam Barati

Comment 7 2017-10-27 16:14:28 PDT

Comment on attachment 325213 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=325213&action=review > Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.cpp:430 > + { > + auto locker = holdLock(m_lock); > + m_liveness = std::make_unique<BytecodeLivenessAnalysis>(codeBlock); > + } Keith pointed out I need to check if we have m_liveness already or not to avoid computing it twice.

Saam Barati

Comment 8 2017-10-27 16:16:10 PDT

Created attachment 325214 [details] patch

Saam Barati

Comment 9 2017-10-27 16:55:14 PDT

Created attachment 325219 [details] patch

Keith Miller

Comment 10 2017-10-27 16:57:08 PDT

Comment on attachment 325219 [details] patch r=me if you get GCC working.

WebKit Commit Bot

Comment 11 2017-10-27 18:03:25 PDT

Comment on attachment 325219 [details] patch Clearing flags on attachment: 325219 Committed r224138: <https://trac.webkit.org/changeset/224138>