V4: tweak Phi node data destruction.

Phi nodes can only occur at the start of a basic block. If there are
any, they need to be subsequent to eachother, and the first phi node
should be the first statement in the basic-block. A number of loops rely
on this behavior, so they don't need to walk through the whole list of
instructions in a basic-block (e.g. the calls to destroyData in
BasicBlock::~BasicBlock).

Change-Id: I57763bc6abae271337b0b169cccd26e10ecd9b2d
Reviewed-by: Lars Knoll <lars.knoll@theqtcompany.com>

2 files changed, 22 insertions, 15 deletions

diff --git a/src/qml/compiler/qv4jsir.cpp b/src/qml/compiler/qv4jsir.cpp
index e383d1432b..a9f8df7748 100644
--- a/src/qml/compiler/qv4jsir.cpp
+++ b/src/qml/compiler/qv4jsir.cpp
@@ -514,9 +514,11 @@ void Function::setStatementCount(int cnt)
 BasicBlock::~BasicBlock()
 {
     for (Stmt *s : qAsConst(_statements)) {
-        Phi *p = s->asPhi();
-        if (p)
+        if (Phi *p = s->asPhi()) {
             p->destroyData();
+        } else {
+            break;
+        }
     }
 }
 
@@ -769,14 +771,15 @@ void BasicBlock::setStatements(const QVector<Stmt *> &newStatements)
 {
     Q_ASSERT(!isRemoved());
     Q_ASSERT(newStatements.size() >= _statements.size());
-    // FIXME: this gets quite inefficient for large basic-blocks, so this function/case should be re-worked.
     for (Stmt *s : qAsConst(_statements)) {
-        Phi *p = s->asPhi();
-        if (!p)
-            continue;
-
-        if (!newStatements.contains(p))
-            p->destroyData();
+        if (Phi *p = s->asPhi()) {
+            if (!newStatements.contains(p)) {
+                // phi-node was not copied over, so:
+                p->destroyData();
+            }
+        } else {
+            break;
+        }
     }
     _statements = newStatements;
 }
@@ -825,27 +828,27 @@ void BasicBlock::insertStatementBeforeTerminator(Stmt *stmt)
 void BasicBlock::replaceStatement(int index, Stmt *newStmt)
 {
     Q_ASSERT(!isRemoved());
-    Phi *p = _statements[index]->asPhi();
-    if (p)
+    if (Phi *p = _statements[index]->asPhi()) {
         p->destroyData();
+    }
     _statements[index] = newStmt;
 }
 
 void BasicBlock::removeStatement(Stmt *stmt)
 {
     Q_ASSERT(!isRemoved());
-    Phi *p = stmt->asPhi();
-    if (p)
+    if (Phi *p = stmt->asPhi()) {
         p->destroyData();
+    }
     _statements.remove(_statements.indexOf(stmt));
 }
 
 void BasicBlock::removeStatement(int idx)
 {
     Q_ASSERT(!isRemoved());
-    Phi *p = _statements[idx]->asPhi();
-    if (p)
+    if (Phi *p = _statements[idx]->asPhi()) {
         p->destroyData();
+    }
     _statements.remove(idx);
 }
 
diff --git a/src/qml/compiler/qv4jsir_p.h b/src/qml/compiler/qv4jsir_p.h
index 0189ff4a23..baa338ed52 100644
--- a/src/qml/compiler/qv4jsir_p.h
+++ b/src/qml/compiler/qv4jsir_p.h
@@ -751,6 +751,10 @@ struct Ret: Stmt {
     virtual Ret *asRet() { return this; }
 };
 
+// Phi nodes can only occur at the start of a basic block. If there are any, they need to be
+// subsequent to eachother, and the first phi node should be the first statement in the basic-block.
+// A number of loops rely on this behavior, so they don't need to walk through the whole list
+// of instructions in a basic-block (e.g. the calls to destroyData in BasicBlock::~BasicBlock).
 struct Phi: Stmt {
     Temp *targetTemp;
     VarLengthArray<Expr *, 4> incoming;