[webkit-reviews] review granted: [Bug 230823] Run backwards propagation before we prune the graph after ForceOSRExit nodes in BytecodeParser : [Attachment 439885] patch

[bugzilla-daemon at webkit.org]

Robin Morisset <rmorisset at apple.com> has granted Saam Barati
<sbarati at apple.com>'s request for review:
Bug 230823: Run backwards propagation before we prune the graph after
ForceOSRExit nodes in BytecodeParser
https://bugs.webkit.org/show_bug.cgi?id=230823

Attachment 439885: patch

https://bugs.webkit.org/attachment.cgi?id=439885&action=review




--- Comment #10 from Robin Morisset <rmorisset at apple.com> ---
Comment on attachment 439885
  --> https://bugs.webkit.org/attachment.cgi?id=439885
patch

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

r=me.
Overall I'm very happy to see that the ugly hack at the end of BytecodeParser
which removes some of the code after OSRExits but not all is at least partly
gone.

> Source/JavaScriptCore/ChangeLog:19
> +	   That way, the phase sees the graph as if its an IR over the whole
bytecode

nit: its => it's

> Source/JavaScriptCore/dfg/DFGBackwardsPropagationPhase.cpp:50
> +	   for (size_t i = 0; i < m_graph.numBlocks(); ++i) {

trivial simplification: you can do
```
for (BasicBlock* block : m_graph.blocksInNaturalOrder()) {
  m_flagsAtHead[block] = ...
```
blocksInNaturalOrder is exactly this pattern of iterating them in order and
skipping any null one.

> Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:9027
> +    performBackwardsPropagation(m_graph);

Would it be possible/safe to flash a GC safe point before this? While backwards
propagation is pretty quick, the bytecode parser is already responsible for
many of our worst GC pauses, so I'd really like to avoid making its impact on
the GC any worse.