V4 interpreter: remove stack-slot allocator.

The life-ranges are only valid when the IR is in SSA form. So the use
of them in the interpreter after converting out of SSA form introduced
bugs. Instead, allocate a stack-slot for each unique temporary, and
re-use the code for this from the JIT.

Change-Id: I294f1116064f0b85996cf96a0b408b41a3c785e2
Reviewed-by: Lars Knoll <lars.knoll@digia.com>

diff --git a/src/qml/compiler/qv4isel_masm.cpp b/src/qml/compiler/qv4isel_masm.cpp
index e49216b..ea2a086 100644 (file)
--- a/src/qml/compiler/qv4isel_masm.cpp
+++ b/src/qml/compiler/qv4isel_masm.cpp
@@ -99,72 +99,6 @@ QV4::ExecutableAllocator::ChunkOfPages *CompilationUnit::chunkForFunction(int fu
 }
 
 namespace {
-class ConvertTemps: protected V4IR::StmtVisitor, protected V4IR::ExprVisitor
-{
-    int _nextFreeStackSlot;
-    QHash<V4IR::Temp, int> _stackSlotForTemp;
-
-    void renumber(V4IR::Temp *t)
-    {
-        if (t->kind != V4IR::Temp::VirtualRegister)
-            return;
-
-        int stackSlot = _stackSlotForTemp.value(*t, -1);
-        if (stackSlot == -1) {
-            stackSlot = _nextFreeStackSlot++;
-            _stackSlotForTemp[*t] = stackSlot;
-        }
-
-        t->kind = V4IR::Temp::StackSlot;
-        t->index = stackSlot;
-    }
-
-public:
-    ConvertTemps()
-        : _nextFreeStackSlot(0)
-    {}
-
-    void toStackSlots(V4IR::Function *function)
-    {
-        _stackSlotForTemp.reserve(function->tempCount);
-
-        foreach (V4IR::BasicBlock *bb, function->basicBlocks)
-            foreach (V4IR::Stmt *s, bb->statements)
-                s->accept(this);
-
-        function->tempCount = _nextFreeStackSlot;
-    }
-
-protected:
-    virtual void visitConst(V4IR::Const *) {}
-    virtual void visitString(V4IR::String *) {}
-    virtual void visitRegExp(V4IR::RegExp *) {}
-    virtual void visitName(V4IR::Name *) {}
-    virtual void visitTemp(V4IR::Temp *e) { renumber(e); }
-    virtual void visitClosure(V4IR::Closure *) {}
-    virtual void visitConvert(V4IR::Convert *e) { e->expr->accept(this); }
-    virtual void visitUnop(V4IR::Unop *e) { e->expr->accept(this); }
-    virtual void visitBinop(V4IR::Binop *e) { e->left->accept(this); e->right->accept(this); }
-    virtual void visitCall(V4IR::Call *e) {
-        e->base->accept(this);
-        for (V4IR::ExprList *it = e->args; it; it = it->next)
-            it->expr->accept(this);
-    }
-    virtual void visitNew(V4IR::New *e) {
-        e->base->accept(this);
-        for (V4IR::ExprList *it = e->args; it; it = it->next)
-            it->expr->accept(this);
-    }
-    virtual void visitSubscript(V4IR::Subscript *e) { e->base->accept(this); e->index->accept(this); }
-    virtual void visitMember(V4IR::Member *e) { e->base->accept(this); }
-    virtual void visitExp(V4IR::Exp *s) { s->expr->accept(this); }
-    virtual void visitMove(V4IR::Move *s) { s->target->accept(this); s->source->accept(this); }
-    virtual void visitJump(V4IR::Jump *) {}
-    virtual void visitCJump(V4IR::CJump *s) { s->cond->accept(this); }
-    virtual void visitRet(V4IR::Ret *s) { s->expr->accept(this); }
-    virtual void visitPhi(V4IR::Phi *) { Q_UNREACHABLE(); }
-};
-
 inline bool isPregOrConst(V4IR::Expr *e)
 {
     if (V4IR::Temp *t = e->asTemp())

diff --git a/src/qml/compiler/qv4isel_moth.cpp b/src/qml/compiler/qv4isel_moth.cpp
index 44b4d66..ccc0b5d 100644 (file)
--- a/src/qml/compiler/qv4isel_moth.cpp
+++ b/src/qml/compiler/qv4isel_moth.cpp
@@ -152,75 +152,6 @@ inline bool isBoolType(V4IR::Expr *e)
 
 } // anonymous namespace
 
-// TODO: extend to optimize out temp-to-temp moves, where the lifetime of one temp ends at that statement.
-//       To handle that, add a hint when such a move will occur, and add a stmt for the hint.
-//       Then when asked for a register, check if the active statement is the terminating statement, and if so, apply the hint.
-//       This generalises the hint usage for Phi removal too, when the phi is passed in there as the current statement.
-class QQmlJS::Moth::StackSlotAllocator
-{
-    QHash<V4IR::Temp, int> _slotForTemp;
-    QHash<V4IR::Temp, int> _hints;
-    QVector<int> _activeSlots;
-
-    QHash<V4IR::Temp, V4IR::LifeTimeInterval> _intervals;
-
-public:
-    StackSlotAllocator(const QVector<V4IR::LifeTimeInterval> &ranges, int maxTempCount)
-        : _activeSlots(maxTempCount)
-    {
-        _intervals.reserve(ranges.size());
-        foreach (const V4IR::LifeTimeInterval &r, ranges)
-            _intervals[r.temp()] = r;
-    }
-
-    void addHint(const V4IR::Temp &hintedSlotOfTemp, const V4IR::Temp &newTemp)
-    {
-        if (hintedSlotOfTemp.kind != V4IR::Temp::VirtualRegister
-                || newTemp.kind != V4IR::Temp::VirtualRegister)
-            return;
-
-        if (_slotForTemp.contains(newTemp) || _hints.contains(newTemp))
-            return;
-
-        int hintedSlot = _slotForTemp.value(hintedSlotOfTemp, -1);
-        Q_ASSERT(hintedSlot >= 0);
-        _hints[newTemp] = hintedSlot;
-    }
-
-    int stackSlotFor(V4IR::Temp *t, V4IR::Stmt *currentStmt) {
-        Q_ASSERT(t->kind == V4IR::Temp::VirtualRegister);
-        Q_ASSERT(t->scope == 0);
-        int idx = _slotForTemp.value(*t, -1);
-        if (idx == -1)
-            idx = allocateSlot(t, currentStmt);
-        Q_ASSERT(idx >= 0);
-        return idx;
-    }
-
-private:
-    int allocateSlot(V4IR::Temp *t, V4IR::Stmt *currentStmt) {
-        Q_ASSERT(currentStmt->id > 0);
-
-        const V4IR::LifeTimeInterval &interval = _intervals[*t];
-        int idx = _hints.value(*t, -1);
-        if (idx != -1 && _activeSlots[idx] == currentStmt->id) {
-            _slotForTemp[*t] = idx;
-            _activeSlots[idx] = interval.end();
-            return idx;
-        }
-
-        for (int i = 0, ei = _activeSlots.size(); i != ei; ++i) {
-            if (_activeSlots[i] < currentStmt->id) {
-                _slotForTemp[*t] = i;
-                _activeSlots[i] = interval.end();
-                return i;
-            }
-        }
-
-        return -1;
-    }
-};
-
 InstructionSelection::InstructionSelection(QV4::ExecutableAllocator *execAllocator, V4IR::Module *module, QV4::Compiler::JSUnitGenerator *jsGenerator)
     : EvalInstructionSelection(execAllocator, module, jsGenerator)
     , _function(0)
@@ -228,7 +159,6 @@ InstructionSelection::InstructionSelection(QV4::ExecutableAllocator *execAllocat
     , _codeStart(0)
     , _codeNext(0)
     , _codeEnd(0)
-    , _stackSlotAllocator(0)
     , _currentStatement(0)
 {
     compilationUnit = new CompilationUnit;
@@ -263,13 +193,12 @@ void InstructionSelection::run(int functionIndex)
 
     V4IR::Optimizer opt(_function);
     opt.run();
-    StackSlotAllocator *stackSlotAllocator = 0;
     if (opt.isInSSA()) {
-        //stackSlotAllocator = new StackSlotAllocator(opt.lifeRanges(), _function->tempCount);
         opt.convertOutOfSSA();
+        opt.showMeTheCode(_function);
     }
+    ConvertTemps().toStackSlots(_function);
 
-    qSwap(_stackSlotAllocator, stackSlotAllocator);
     QSet<V4IR::Jump *> removableJumps = opt.calculateOptionalJumps();
     qSwap(_removableJumps, removableJumps);
 
@@ -324,8 +253,6 @@ void InstructionSelection::run(int functionIndex)
 
     qSwap(_currentStatement, cs);
     qSwap(_removableJumps, removableJumps);
-    qSwap(_stackSlotAllocator, stackSlotAllocator);
-    delete stackSlotAllocator;
     qSwap(_function, function);
     qSwap(block, _block);
     qSwap(nextBlock, _nextBlock);
@@ -395,9 +322,6 @@ void InstructionSelection::callSubscript(V4IR::Expr *base, V4IR::Expr *index, V4
 
 void InstructionSelection::convertType(V4IR::Temp *source, V4IR::Temp *target)
 {
-    if (_stackSlotAllocator)
-        _stackSlotAllocator->addHint(*source, *target);
-
     // FIXME: do something more useful with this info
     if (target->type & V4IR::NumberType)
         unop(V4IR::OpUPlus, source, target);
@@ -615,9 +539,6 @@ void InstructionSelection::setElement(V4IR::Expr *source, V4IR::Expr *targetBase
 
 void InstructionSelection::copyValue(V4IR::Temp *sourceTemp, V4IR::Temp *targetTemp)
 {
-    if (_stackSlotAllocator)
-        _stackSlotAllocator->addHint(*sourceTemp, *targetTemp);
-
     Instruction::Move move;
     move.source = getParam(sourceTemp);
     move.result = getResultParam(targetTemp);
@@ -635,9 +556,6 @@ void InstructionSelection::swapValues(V4IR::Temp *sourceTemp, V4IR::Temp *target
 
 void InstructionSelection::unop(V4IR::AluOp oper, V4IR::Temp *sourceTemp, V4IR::Temp *targetTemp)
 {
-    if (_stackSlotAllocator)
-        _stackSlotAllocator->addHint(*sourceTemp, *targetTemp);
-
     switch (oper) {
     case V4IR::OpIfTrue:
         Q_ASSERT(!"unreachable"); break;
@@ -751,9 +669,6 @@ Param InstructionSelection::binopHelper(V4IR::AluOp oper, V4IR::Expr *leftSource
         }
     }
 
-    if (_stackSlotAllocator && target && leftSource->asTemp())
-        _stackSlotAllocator->addHint(*leftSource->asTemp(), *target);
-
     if (oper == V4IR::OpAdd) {
         Instruction::Add add;
         add.lhs = getParam(leftSource);
@@ -1249,11 +1164,10 @@ Param InstructionSelection::getParam(V4IR::Expr *e) {
         case V4IR::Temp::ScopedFormal: return Param::createArgument(t->index, t->scope);
         case V4IR::Temp::Local: return Param::createLocal(t->index);
         case V4IR::Temp::ScopedLocal: return Param::createScopedLocal(t->index, t->scope);
-        case V4IR::Temp::VirtualRegister:
-            return Param::createTemp(_stackSlotAllocator ?
-                        _stackSlotAllocator->stackSlotFor(t, _currentStatement) : t->index);
+        case V4IR::Temp::StackSlot:
+            return Param::createTemp(t->index);
         default:
-            Q_UNIMPLEMENTED();
+            Q_UNREACHABLE();
             return Param();
         }
     } else {

diff --git a/src/qml/compiler/qv4isel_moth_p.h b/src/qml/compiler/qv4isel_moth_p.h
index 0bf7444..3a2fdb0 100644 (file)
--- a/src/qml/compiler/qv4isel_moth_p.h
+++ b/src/qml/compiler/qv4isel_moth_p.h
@@ -54,8 +54,6 @@ QT_BEGIN_NAMESPACE
 namespace QQmlJS {
 namespace Moth {
 
-class StackSlotAllocator;
-
 struct CompilationUnit : public QV4::CompiledData::CompilationUnit
 {
     virtual ~CompilationUnit();
@@ -65,7 +63,6 @@ struct CompilationUnit : public QV4::CompiledData::CompilationUnit
 
 };
 
-
 class Q_QML_EXPORT InstructionSelection:
         public V4IR::IRDecoder,
         public EvalInstructionSelection
@@ -182,7 +179,6 @@ private:
     uchar *_codeNext;
     uchar *_codeEnd;
 
-    StackSlotAllocator *_stackSlotAllocator;
     QSet<V4IR::Jump *> _removableJumps;
     V4IR::Stmt *_currentStatement;
 

diff --git a/src/qml/compiler/qv4isel_util_p.h b/src/qml/compiler/qv4isel_util_p.h
index 5084de8..38ea682 100644 (file)
--- a/src/qml/compiler/qv4isel_util_p.h
+++ b/src/qml/compiler/qv4isel_util_p.h
@@ -95,6 +95,71 @@ inline QV4::Primitive convertToValue(V4IR::Const *c)
     return QV4::Primitive::undefinedValue();
 }
 
+class ConvertTemps: protected V4IR::StmtVisitor, protected V4IR::ExprVisitor
+{
+    int _nextFreeStackSlot;
+    QHash<V4IR::Temp, int> _stackSlotForTemp;
+
+    void renumber(V4IR::Temp *t)
+    {
+        if (t->kind != V4IR::Temp::VirtualRegister)
+            return;
+
+        int stackSlot = _stackSlotForTemp.value(*t, -1);
+        if (stackSlot == -1) {
+            stackSlot = _nextFreeStackSlot++;
+            _stackSlotForTemp[*t] = stackSlot;
+        }
+
+        t->kind = V4IR::Temp::StackSlot;
+        t->index = stackSlot;
+    }
+
+public:
+    ConvertTemps()
+        : _nextFreeStackSlot(0)
+    {}
+
+    void toStackSlots(V4IR::Function *function)
+    {
+        _stackSlotForTemp.reserve(function->tempCount);
+
+        foreach (V4IR::BasicBlock *bb, function->basicBlocks)
+            foreach (V4IR::Stmt *s, bb->statements)
+                s->accept(this);
+
+        function->tempCount = _nextFreeStackSlot;
+    }
+
+protected:
+    virtual void visitConst(V4IR::Const *) {}
+    virtual void visitString(V4IR::String *) {}
+    virtual void visitRegExp(V4IR::RegExp *) {}
+    virtual void visitName(V4IR::Name *) {}
+    virtual void visitTemp(V4IR::Temp *e) { renumber(e); }
+    virtual void visitClosure(V4IR::Closure *) {}
+    virtual void visitConvert(V4IR::Convert *e) { e->expr->accept(this); }
+    virtual void visitUnop(V4IR::Unop *e) { e->expr->accept(this); }
+    virtual void visitBinop(V4IR::Binop *e) { e->left->accept(this); e->right->accept(this); }
+    virtual void visitCall(V4IR::Call *e) {
+        e->base->accept(this);
+        for (V4IR::ExprList *it = e->args; it; it = it->next)
+            it->expr->accept(this);
+    }
+    virtual void visitNew(V4IR::New *e) {
+        e->base->accept(this);
+        for (V4IR::ExprList *it = e->args; it; it = it->next)
+            it->expr->accept(this);
+    }
+    virtual void visitSubscript(V4IR::Subscript *e) { e->base->accept(this); e->index->accept(this); }
+    virtual void visitMember(V4IR::Member *e) { e->base->accept(this); }
+    virtual void visitExp(V4IR::Exp *s) { s->expr->accept(this); }
+    virtual void visitMove(V4IR::Move *s) { s->target->accept(this); s->source->accept(this); }
+    virtual void visitJump(V4IR::Jump *) {}
+    virtual void visitCJump(V4IR::CJump *s) { s->cond->accept(this); }
+    virtual void visitRet(V4IR::Ret *s) { s->expr->accept(this); }
+    virtual void visitPhi(V4IR::Phi *) { Q_UNREACHABLE(); }
+};
 } // namespace QQmlJS
 
 QT_END_NAMESPACE