<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body><span class="vcard"><a class="email" href="mailto:ggaren@apple.com" title="Geoffrey Garen <ggaren@apple.com>"> <span class="fn">Geoffrey Garen</span></a>
</span> changed
<a class="bz_bug_link
bz_status_NEW "
title="NEW - NewRegexp should not prevent inlining"
href="https://bugs.webkit.org/show_bug.cgi?id=154808">bug 154808</a>
<br>
<table border="1" cellspacing="0" cellpadding="8">
<tr>
<th>What</th>
<th>Removed</th>
<th>Added</th>
</tr>
<tr>
<td style="text-align:right;">Attachment #286601 Flags</td>
<td>review?
</td>
<td>review-
</td>
</tr></table>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - NewRegexp should not prevent inlining"
href="https://bugs.webkit.org/show_bug.cgi?id=154808#c6">Comment # 6</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - NewRegexp should not prevent inlining"
href="https://bugs.webkit.org/show_bug.cgi?id=154808">bug 154808</a>
from <span class="vcard"><a class="email" href="mailto:ggaren@apple.com" title="Geoffrey Garen <ggaren@apple.com>"> <span class="fn">Geoffrey Garen</span></a>
</span></b>
<pre>Comment on <span class=""><a href="attachment.cgi?id=286601&action=diff" name="attach_286601" title="Patch">attachment 286601</a> <a href="attachment.cgi?id=286601&action=edit" title="Patch">[details]</a></span>
Patch
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=286601&action=review">https://bugs.webkit.org/attachment.cgi?id=286601&action=review</a>
I think a better model might be for every CodeBlock always to copy its RegExp array out of its unlinked counterpart and into a unique vector. This will waste a little space, but we think it's rare, so it's probably acceptable. The nice thing about this change is that we won't need two different modes for accessing regexps.
If we do need two different modes to save space, I would call the first mode "unlinkedRegExp()" and the second mode "jitCodeRegExp()", and I would store the regexeps used only by optimizing compilers on the JITCode object.
<span class="quote">> Source/JavaScriptCore/ChangeLog:9
> + when it contains NewRegExp nodes. In current implementation, it is not</span >
when the callee contains
In the current implementation, inlining
<span class="quote">> Source/JavaScriptCore/ChangeLog:10
> + possible because the RegExp references are being stored in their function's</span >
are stored
<span class="quote">> Source/JavaScriptCore/ChangeLog:12
> + NewRegExp node is emmited with the RegExp's index in CodeBlock vector</span >
emitted
in the CodeBlock vector
<span class="quote">> Source/JavaScriptCore/ChangeLog:13
> + as parameter to be retrived in futher phases of compilation</span >
as a parameter
retrieved
<span class="quote">> Source/JavaScriptCore/ChangeLog:16
> + As solution, we are remmaping the RegExps from its callee inline canditate</span >
As a solution, we remap each RegExp
candidate
<span class="quote">> Source/JavaScriptCore/ChangeLog:17
> + to its machine code block in inline phase into</span >
in the inlining phase
<span class="quote">> Source/JavaScriptCore/ChangeLog:24
> + It is important notice that we created a new Vector because</span >
to notice
<span class="quote">> Source/JavaScriptCore/ChangeLog:27
> + out of compilation thread.</span >
from the compilation thread
<span class="quote">> Source/JavaScriptCore/bytecode/CodeBlock.h:557
> + unsigned addRegExpList(RegExp* regexp)</span >
This function name sounds like you're adding a list of RegExps. I don't think it differs clearly enough from CodeBlock::regexp(). It is a little surprising that CodeBlock has two different RegExp stores, one that forwards to the unlinked code block and one that is local.
<span class="quote">> Source/JavaScriptCore/bytecode/CodeBlock.h:900
> + Vector<RegExp*> m_regExps;</span >
Why is it OK to store raw pointers to RegExp without a write barrier operation? You've explained why we can't do write barrier operations on the compilation thread, but I don't think you've explained why it's OK just to skip them -- and I don't think it is OK.
<span class="quote">> Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:5401
> + </span >
Revert.
<span class="quote">> Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:5422
> + </span >
Revert.
<span class="quote">> Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:5428
> + // We need to remap and copy RegExps from codeBlock->unlinkedCodeBlock() to
> + // byteCodeParser->m_codeBlock because it allow us inline functions with NewRegExp.
> + // nodes.</span >
I don't fully understand this comment.</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>