V4 regalloc: fix interval splitting when register pressure is high.
authorErik Verbruggen <erik.verbruggen@digia.com>
Thu, 30 Jan 2014 12:48:31 +0000 (13:48 +0100)
committerThe Qt Project <gerrit-noreply@qt-project.org>
Fri, 31 Jan 2014 09:34:36 +0000 (10:34 +0100)
When a register is needed for an interval, but none is available, one
of the used registers will be chosen and spilled to the stack (meaning
it will be split) before the current position. However, if a register
is used at the current position for an incoming parameter, its interval
has to be split after the current position. This patch adds/fixes the
latter case.

This fixes crypto.js on x86. The specific problem there was that the
result of an operation needed a register, and chose the one from one
of the incoming parameters (which then should get spilled/split).
However, this interval was already split, and started exactly at that
the current position. So splitting before the current position did
nothing, resulting in it staying alive and using the same register as
the result. So any subsequent use of would have the invalid value.

Task-number: QTBUG-36430
Change-Id: I228fc210b009aa0b16b08a374fc955fabfbb6d12
Reviewed-by: Albert Astals Cid <albert.astals@canonical.com>
Reviewed-by: Simon Hausmann <simon.hausmann@digia.com>
Reviewed-by: Fawzi Mohamed <fawzi.mohamed@digia.com>
Reviewed-by: Lars Knoll <lars.knoll@digia.com>
src/qml/compiler/qv4regalloc.cpp

index a3255f2..ea2338a 100644 (file)
@@ -111,6 +111,13 @@ public:
         return false;
     }
 
+    bool isUsedAt(const Temp &t, int position) {
+        foreach (const Use &use, uses(t))
+            if (use.pos == position)
+                return true;
+        return false;
+    }
+
     int def(const Temp &t) const {
         Q_ASSERT(_defs[t].isValid());
         return _defs[t].defStmt;
@@ -743,17 +750,17 @@ private:
             os << "Intervals live at the start of L" << bb->index << ":" << endl;
             if (_liveAtStart[bb].isEmpty())
                 os << "\t(none)" << endl;
-            foreach (const LifeTimeInterval &i, _liveAtStart[bb]) {
+            foreach (const LifeTimeInterval *i, _liveAtStart[bb]) {
                 os << "\t";
-                i.dump(os);
+                i->dump(os);
                 os << endl;
             }
             os << "Intervals live at the end of L" << bb->index << ":" << endl;
             if (_liveAtEnd[bb].isEmpty())
                 os << "\t(none)" << endl;
-            foreach (const LifeTimeInterval &i, _liveAtEnd[bb]) {
+            foreach (const LifeTimeInterval *i, _liveAtEnd[bb]) {
                 os << "\t";
-                i.dump(os);
+                i->dump(os);
                 os << endl;
             }
 #endif
@@ -1066,6 +1073,8 @@ RegisterAllocator::RegisterAllocator(const QVector<int> &normalRegisters, const
     : _normalRegisters(normalRegisters)
     , _fpRegisters(fpRegisters)
 {
+    Q_ASSERT(normalRegisters.size() >= 2);
+    Q_ASSERT(fpRegisters.size() >= 2);
 }
 
 RegisterAllocator::~RegisterAllocator()
@@ -1413,9 +1422,20 @@ void RegisterAllocator::allocateBlockedReg(LifeTimeInterval &current, const int
 #endif // DEBUG_REGALLOC
         current.setReg(reg);
         _lastAssignedRegister.insert(current.temp(), reg);
-        Q_ASSERT(nextUseRangeForReg[reg]);
-        Q_ASSERT(!nextUseRangeForReg[reg]->isFixedInterval());
-        split(*nextUseRangeForReg[reg], position);
+        LifeTimeInterval *nextUse = nextUseRangeForReg[reg];
+        Q_ASSERT(nextUse);
+        Q_ASSERT(!nextUse->isFixedInterval());
+
+        if (_info->isUsedAt(nextUse->temp(), position)) {
+            Q_ASSERT(!_info->isUsedAt(current.temp(), position));
+            // the register is used (as an incoming parameter) at the current position, so split
+            // the interval immediately after the (use at the) current position
+            split(*nextUse, position + 1);
+        } else {
+            // the register was used before the current position
+            split(*nextUse, position);
+        }
+
         splitInactiveAtEndOfLifetimeHole(reg, needsFPReg, position);
 
         // make sure that current does not intersect with the fixed interval for reg