WebKit Bugzilla
New
Browse
Search+
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
[patch]
proposed patch
dce-bug-130115.diff (text/plain), 5.79 KB, created by
Gergő Balogh
on 2014-08-29 00:03:29 PDT
(
hide
)
Description:
proposed patch
Filename:
MIME Type:
Creator:
Gergő Balogh
Created:
2014-08-29 00:03:29 PDT
Size:
5.79 KB
patch
obsolete
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog >index 0692853..07c1eb8 100644 >--- a/Source/JavaScriptCore/ChangeLog >+++ b/Source/JavaScriptCore/ChangeLog >@@ -1,3 +1,16 @@ >+2014-08-28 Gergo Balogh <gbalogh.u-szeged@partner.samsung.com> >+ >+ DCE claims to preserve CPS but does not preserve it in the strictest form, specifically variables-at-tail for captured variables. >+ Nhttps://bugs.webkit.org/show_bug.cgi?id=130115 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ * dfg/DFGDCEPhase.cpp: >+ (JSC::DFG::DCEPhase::fixupBlock): >+ (JSC::DFG::DCEPhase::cleanVariables): >+ (JSC::DFG::DCEPhase::cleanVariablesAtTail): >+ (JSC::DFG::performDCE): Deleted. >+ > 2014-08-26 Brian J. Burg <burg@cs.washington.edu> > > Web Inspector: put feature flags for Inspector domains in the protocol specification >diff --git a/Source/JavaScriptCore/dfg/DFGDCEPhase.cpp b/Source/JavaScriptCore/dfg/DFGDCEPhase.cpp >index a69eb00..365b149 100644 >--- a/Source/JavaScriptCore/dfg/DFGDCEPhase.cpp >+++ b/Source/JavaScriptCore/dfg/DFGDCEPhase.cpp >@@ -210,7 +210,7 @@ private: > } > > cleanVariables(block->variablesAtHead); >- cleanVariables(block->variablesAtTail); >+ cleanVariablesAtTail(block, block->variablesAtTail); > break; > } > >@@ -285,10 +285,27 @@ private: > continue; > if (node->op() != Phantom && node->op() != Check && node->shouldGenerate()) > continue; >+ RELEASE_ASSERT(node->op() != GetLocal); >+ variables[i] = 0; >+ } >+ } >+ >+ template<typename VariablesVectorType> >+ void cleanVariablesAtTail(BasicBlock* block, VariablesVectorType& variables) >+ { >+ bool recalcVars = false; >+ BitVector varsToRecalc; >+ >+ for (unsigned i = variables.size(); i--;) { >+ Node* node = variables[i]; >+ if (!node) >+ continue; >+ if (node->op() != Phantom && node->shouldGenerate()) >+ continue; > if (node->op() == GetLocal) { > node = node->child1().node(); >- >- // FIXME: In the case that the variable is captured, we really want to be able >+ >+ // In the case that the variable is captured, we really want to be able > // to replace the variable-at-tail with the last use of the variable in the same > // way that CPS rethreading would do. The child of the GetLocal isn't necessarily > // the same as what CPS rethreading would do. For example, we may have: >@@ -297,36 +314,51 @@ private: > // b: GetLocal(@a) // live > // c: GetLocal(@a) // dead > // >- // When killing @c, the code below will set the variable-at-tail to @a, while CPS >- // rethreading would have set @b. This is a benign bug, since all clients of CPS >- // only use the variable-at-tail of captured variables to get the >- // VariableAccessData and observe that it is in fact captured. But, this feels >- // like it could cause bugs in the future. >- // >- // It's tempting to just dethread and then invoke CPS rethreading, but CPS >- // rethreading fails to preserve exact ref-counts. So we would need a fixpoint. >- // It's probably the case that this fixpoint will be guaranteed to converge after >- // the second iteration (i.e. the second run of DCE will not kill anything and so >- // will not need to dethread), but for now the safest approach is probably just to >- // allow for this tiny bit of sloppiness. >- // >- // Another possible solution would be to simply say that DCE dethreads but then >- // we never rethread before going to the backend. That feels intuitively right >- // because it's unlikely that any of the phases after DCE in the backend rely on >- // ThreadedCPS. >- // >- // https://bugs.webkit.org/show_bug.cgi?id=130115 >+ // When killing @c, variables[i] = node; in the code below will set the >+ // variable-at-tail to @a, while CPS rethreading would have set @b. >+ // Thus, in such cases we iterate through all the nodes of the block and mimic >+ // CPS rethreading more closely. > ASSERT( > node->op() == Phi || node->op() == SetArgument > || node->variableAccessData()->isCaptured()); > > if (node->shouldGenerate()) { >+ if (!recalcVars) { >+ recalcVars = true; >+ varsToRecalc.ensureSize(variables.size()); >+ } >+ varsToRecalc.quickSet(i); >+ > variables[i] = node; > continue; > } > } > variables[i] = 0; > } >+ >+ if (!recalcVars) >+ return; >+ >+ for (unsigned nodeIndex = 0; nodeIndex < block->size(); ++nodeIndex) { >+ Node *node = block->at(nodeIndex); >+ if (!node->shouldGenerate()) >+ continue; >+ >+ switch (node->op()) { >+ case SetArgument: >+ case SetLocal: >+ case GetLocal: >+ { >+ unsigned varIndex = variables.indexForOperand(node->local()); >+ if (varsToRecalc.quickGet(varIndex)) >+ variables[varIndex] = node; >+ break; >+ } >+ default: >+ break; >+ } >+ >+ } > } > > Vector<Node*, 128> m_worklist;
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 130115
:
237337
|
237338