Attachment 271683 Details for Bug 154405 – it's a start

[patch] it's a start

blah.patch (text/plain), 9.23 KB, created by Filip Pizlo on 2016-02-18 12:41:37 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Filip Pizlo

Created: 2016-02-18 12:41:37 PST

Size: 9.23 KB

patch

obsolete

>Index: Source/JavaScriptCore/b3/air/AirIteratedRegisterCoalescing.cpp
>===================================================================
>--- Source/JavaScriptCore/b3/air/AirIteratedRegisterCoalescing.cpp	(revision 196765)
>+++ Source/JavaScriptCore/b3/air/AirIteratedRegisterCoalescing.cpp	(working copy)
>@@ -751,9 +751,20 @@ public:
>     // This tells you if a Move will be coalescable if the src and dst end up matching. This method
>     // relies on an analysis that is invalidated by register allocation, so you it's only meaningful to
>     // call this *before* replacing the Tmp's in this Inst with registers or spill slots.
>-    bool mayBeCoalescable(const Inst& inst) const
>+    bool mayBeCoalescableMove(const Inst& inst) const
>     {
>-        return mayBeCoalescableImpl(inst, &m_tmpWidth);
>+        return mayBeCoalescableMoveImpl(inst, &m_tmpWidth);
>+    }
>+
>+    bool mayBeCoalescableShufflePair(const Inst& inst, unsigned index) const
>+    {
>+        return mayBeCoalescable(
>+            inst.args[index + 0].width(), inst.args[index + 1], inst.args[index + 2]);
>+    }
>+
>+    bool mayBeCoalescable(Arg::Width width, const Arg& src, const Arg& dest)
>+    {
>+        return mayBeCoalescableImpl(width, src, dest, &m_tmpWidth);
>     }
> 
>     bool isUselessMove(const Inst& inst) const
>@@ -761,6 +772,17 @@ public:
>         return mayBeCoalescableImpl(inst, nullptr) && inst.args[0].tmp() == inst.args[1].tmp();
>     }
> 
>+    bool isUselessShufflePair(const Inst& inst, unsigned index) const
>+    {
>+        return isUselessMove(
>+            inst.args[index + 0].width(), inst.args[index + 1], inst.args[index + 2]);
>+    }
>+
>+    bool isUselessMove(Arg::Width width, const Arg& src, const Arg& dest)
>+    {
>+        return mayBeCoalescableImpl(width, src, dest, nullptr) && src.tmp() == dest.tmp();
>+    }
>+
>     Tmp getAliasWhenSpilling(Tmp tmp) const
>     {
>         ASSERT_WITH_MESSAGE(!m_spilledTmps.isEmpty(), "This function is only valid for coalescing during spilling.");
>@@ -941,22 +963,79 @@ private:
>                     });
>             });
> 
>-        if (prevInst && mayBeCoalescable(*prevInst)) {
>-            // We do not want the Use() of this move to interfere with the Def(), even if it is live
>+        if (prevInst) {
>+            // This code path attempts to handle coalescable instructions. It handles both Moves and
>+            // Shuffles. The trick is to ensure that the definitions in a Move don't interfere with
>+            // the sources, so if we detect a coalescing opportunity we also avoid taking the usual
>+            // interference path by doing the interference calculation manually and returning early.
>+            
>+            auto addCoalescableMove = [&] (Tmp srcTmp, Tmp destTmp) {
>+                unsigned srcIndex = AbsoluteTmpMapper<type>::absoluteIndex(srcTmp);
>+                unsigned destIndex = AbsoluteTmpMapper<type>::absoluteIndex(destTmp);
>+                
>+                unsigned nextMoveIndex = m_coalescingCandidates.size();
>+                m_coalescingCandidates.append({ srcIndex, destIndex });
>+                
>+                unsigned newIndexInWorklist = m_worklistMoves.addMove();
>+                ASSERT_UNUSED(newIndexInWorklist, newIndexInWorklist == nextMoveIndex);
>+                
>+                ASSERT(nextMoveIndex <= m_activeMoves.size());
>+                m_activeMoves.ensureSize(nextMoveIndex + 1);
>+
>+                m_moveList[srcIndex].add(nextMoveIndex);
>+                m_moveList[destIndex].add(nextMoveIndex);
>+            };
>+            
>+            if (mayBeCoalescableMove(*prevInst)) {
>+                Tmp srcTmp = prevInst->args[0].tmp();
>+                Tmp destTmp = prevInst->args[1].tmp();
>+                addCoalescableMove(srcTmp, destTmp);
>+
>+                for (Tmp liveTmp : localCalc.live()) {
>+                    if (liveTmp != srcTmp)
>+                        addEdge(destTmp, liveTmp);
>+                }
>+
>+                // The next instruction cpuld have early clobbers or early def's. Consider those
>+                // now.
>+                addEdges(nullptr, nextInst, localCalc.live());
>+                return;
>+            }
>+
>+            if (prevInst->opcode() == Shuffle) {
>+                for (unsigned i = 0; i < prevInst->args.size(); i += 3) {
>+                    Arg dest = prevInst->args[i + 2];
>+                    
>+                    if (mayBeCoalescableShufflePair(*prevInst, i)) {
>+                        Tmp srcTmp = prevInst->args[i + 1].tmp();
>+                        Tmp destTmp = dest.tmp();
>+                        addCoalescableMove(srcTmp, destTemp);
>+
>+                        // Conservatively, dest would interfere with everything live. But in this
>+                        // case it only interferes with all live variables except srcTmp.
>+                        for (Tmp liveTmp : localCalc.live()) {
>+                            if (liveTmp != srcTmp)
>+                                addEdge(destTmp, liveTmp);
>+                        }
>+                        continue;
>+                    }
>+
>+                    // If the destination is a tmp, it needs to interfere with everything live.
>+                    if (dest.isTmp()) {
>+                        for (Tmp liveTmp : localCalc.live())
>+                            addEdge(dest.tmp(), liveTmp);
>+                    }
>+                }
>+            }
>+        }
>+
>+        if (prevInst && mayBeCoalescableMove(*prevInst)) {
>+            Tmp srcTmp = prevInst->args[0];
>+            Tmp defTmp = prevInst->args[1];
>+
>+            // We do not want the Use of this move to interfere with the Def, even if it is live
>             // after the Move. If we were to add the interference edge, it would be impossible to
>             // coalesce the Move even if the two Tmp never interfere anywhere.
>-            Tmp defTmp;
>-            Tmp useTmp;
>-            prevInst->forEachTmp([&defTmp, &useTmp] (Tmp& argTmp, Arg::Role role, Arg::Type, Arg::Width) {
>-                if (Arg::isLateDef(role))
>-                    defTmp = argTmp;
>-                else {
>-                    ASSERT(Arg::isEarlyUse(role));
>-                    useTmp = argTmp;
>-                }
>-            });
>-            ASSERT(defTmp);
>-            ASSERT(useTmp);
> 
>             unsigned nextMoveIndex = m_coalescingCandidates.size();
>             m_coalescingCandidates.append({ AbsoluteTmpMapper<type>::absoluteIndex(useTmp), AbsoluteTmpMapper<type>::absoluteIndex(defTmp) });
>@@ -1037,13 +1116,18 @@ private:
> 
>     // Calling this without a tmpWidth will perform a more conservative coalescing analysis that assumes
>     // that Move32's are not coalescable.
>-    static bool mayBeCoalescableImpl(const Inst& inst, TmpWidth* tmpWidth)
>+    static bool mayBeCoalescableMoveImpl(const Inst& inst, TmpWidth* tmpWidth)
>     {
>+        Arg::Width width;
>+        
>         switch (type) {
>         case Arg::GP:
>             switch (inst.opcode) {
>             case Move:
>+                width = Arg::conservativeWidth(type);
>+                break;
>             case Move32:
>+                width = Arg::Width32;
>                 break;
>             default:
>                 return false;
>@@ -1052,7 +1136,10 @@ private:
>         case Arg::FP:
>             switch (inst.opcode) {
>             case MoveFloat:
>+                width = Arg::Width32;
>+                break;
>             case MoveDouble:
>+                width = Arg::Width64;
>                 break;
>             default:
>                 return false;
>@@ -1062,29 +1149,32 @@ private:
> 
>         ASSERT_WITH_MESSAGE(inst.args.size() == 2, "We assume coalecable moves only have two arguments in a few places.");
> 
>-        if (!inst.args[0].isTmp() || !inst.args[1].isTmp())
>+        return mayBeCoalescableImpl(width, inst.args[0], inst.args[1], tmpWidth);
>+    }
>+
>+    static bool mayBeCoalescableImpl(
>+        Arg::Width width, const Arg& src, const Arg& dest, TmpWidth* tmpWidth)
>+    {
>+        if (!src.isTmp() || !dest.isTmp())
>             return false;
> 
>-        ASSERT(inst.args[0].type() == type);
>-        ASSERT(inst.args[1].type() == type);
>+        if (!src.isType(type))
>+            return false;
> 
>-        // We can coalesce a Move32 so long as either of the following holds:
>-        // - The input is already zero-filled.
>-        // - The output only cares about the low 32 bits.
>-        //
>-        // Note that the input property requires an analysis over ZDef's, so it's only valid so long
>-        // as the input gets a register. We don't know if the input gets a register, but we do know
>-        // that if it doesn't get a register then we will still emit this Move32.
>-        if (inst.opcode == Move32) {
>-            if (!tmpWidth)
>-                return false;
>+        ASSERT(width == Width32 || width == Width64);
>+        ASSERT(src.type() == dest.type());
> 
>-            if (tmpWidth->defWidth(inst.args[0].tmp()) > Arg::Width32
>-                && tmpWidth->useWidth(inst.args[1].tmp()) > Arg::Width32)
>-                return false;
>-        }
>-        
>-        return true;
>+        if (type == Arg::FP)
>+            return true;
>+
>+        if (width == Arg::conservativeWidth(type))
>+            return true;
>+
>+        if (!tmpWidth)
>+            return false;
>+
>+        return tmpWidth->defWidth(src) <= Arg::Width32
>+            || tmpWidth->useWidth(dest) <= Arg::Width32;
>     }
> 
>     void selectSpill()

Actions: View | Formatted Diff | Diff

Attachments on bug 154405: 271683 | 271710 | 271719 | 272036 | 272628