Created attachment 267183 [details] the patch

Attachment 267183 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/b3/B3ReduceStrength.cpp:41: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 1 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.

Comment on attachment 267183 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=267183&action=review > Source/JavaScriptCore/b3/B3ReduceStrength.cpp:112 > + m_dominators = &m_proc.dominators(); // Recompute if necessary. This is a tiny bit scary :) > Source/JavaScriptCore/b3/B3Value.cpp:448 > + case BitwiseCast: Oops

(In reply to comment #3) > Comment on attachment 267183 [details] > the patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=267183&action=review > > > Source/JavaScriptCore/b3/B3ReduceStrength.cpp:112 > > + m_dominators = &m_proc.dominators(); // Recompute if necessary. > > This is a tiny bit scary :) > > > Source/JavaScriptCore/b3/B3Value.cpp:448 > > + case BitwiseCast: > > Oops Why oops? BitwiseCast is a pure value, and so should have a value key.

(In reply to comment #4) > (In reply to comment #3) > > Comment on attachment 267183 [details] > > the patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=267183&action=review > > > > > Source/JavaScriptCore/b3/B3ReduceStrength.cpp:112 > > > + m_dominators = &m_proc.dominators(); // Recompute if necessary. > > > > This is a tiny bit scary :) > > > > > Source/JavaScriptCore/b3/B3Value.cpp:448 > > > + case BitwiseCast: > > > > Oops > > Why oops? BitwiseCast is a pure value, and so should have a value key. Never mind. :-)

(In reply to comment #3) > Comment on attachment 267183 [details] > the patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=267183&action=review > > > Source/JavaScriptCore/b3/B3ReduceStrength.cpp:112 > > + m_dominators = &m_proc.dominators(); // Recompute if necessary. > > This is a tiny bit scary :) What is scary about it? The cost? It's true that CSE adds cost to reduceStrength. About half of that cost is dominators. Interestingly, this code doesn't need a dominator tree; it only wants dominance queries. For most CFGs, computing dominance using the data flow fixpoint is cheaper than the Lengauer-Tarjan algorithm that we use, so long as you just want dominance queries and not the tree. So, we could reduce that cost if we felt that we needed to. But half of the cost of the CSE is just the HashMap, of course. ;-) > > > Source/JavaScriptCore/b3/B3Value.cpp:448 > > + case BitwiseCast: > > Oops

Landed in http://trac.webkit.org/changeset/193987