FTL B3 should be able to run richards
authorfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 23 Dec 2015 01:50:19 +0000 (01:50 +0000)
committerfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 23 Dec 2015 01:50:19 +0000 (01:50 +0000)
https://bugs.webkit.org/show_bug.cgi?id=152514

Reviewed by Michael Saboff.

Source/JavaScriptCore:

This came down to a liveness bug and a register allocation bug.

The liveness bug was that the code that determined whether we should go around the fixpoint
assumed that BitVector::quickSet() would return true if the bit changed state from false to
true. That's not how it works. It returns the old value of the bit, so it will return false
if the bit changed from false to true. Since there is already a lot of code that relies on
this behavior, I fixed Liveness instead of changing BitVector.

The register allocation bug was that we weren't guarding some checks of tmp()'s with checks
that the Arg isTmp().

The liveness took a long time to track down, and I needed to add a lot of dumping to do it.
It's now possible to dump more of the liveness states, including liveAtHead. I found this
extremely helpful, so I removed the code that cleared liveAtHead.

* b3/air/AirIteratedRegisterCoalescing.cpp:
* b3/air/AirLiveness.h:
(JSC::B3::Air::AbstractLiveness::AbstractLiveness):
(JSC::B3::Air::AbstractLiveness::Iterable::Iterable):
(JSC::B3::Air::AbstractLiveness::Iterable::iterator::iterator):
(JSC::B3::Air::AbstractLiveness::Iterable::iterator::operator*):
(JSC::B3::Air::AbstractLiveness::Iterable::iterator::operator++):
(JSC::B3::Air::AbstractLiveness::Iterable::iterator::operator==):
(JSC::B3::Air::AbstractLiveness::Iterable::iterator::operator!=):
(JSC::B3::Air::AbstractLiveness::Iterable::begin):
(JSC::B3::Air::AbstractLiveness::Iterable::end):
(JSC::B3::Air::AbstractLiveness::liveAtHead):
(JSC::B3::Air::AbstractLiveness::liveAtTail):
* b3/air/AirStackSlot.h:
(WTF::printInternal):
* ftl/FTLOSRExitCompiler.cpp:
(JSC::FTL::compileFTLOSRExit):

Source/WTF:

Change the list dumping helpers to work with a broader set of list kinds.

* wtf/ListDump.h:
(WTF::ListDump::dump):
(WTF::MapDump::dump):
(WTF::sortedMapDump):
(WTF::ListDumpInContext::dump):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@194382 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/b3/air/AirIteratedRegisterCoalescing.cpp
Source/JavaScriptCore/b3/air/AirLiveness.h
Source/JavaScriptCore/b3/air/AirStackSlot.h
Source/JavaScriptCore/ftl/FTLOSRExitCompiler.cpp
Source/WTF/ChangeLog
Source/WTF/wtf/ListDump.h

index 3ccaa74..b054004 100644 (file)
@@ -1,3 +1,43 @@
+2015-12-22  Filip Pizlo  <fpizlo@apple.com>
+
+        FTL B3 should be able to run richards
+        https://bugs.webkit.org/show_bug.cgi?id=152514
+
+        Reviewed by Michael Saboff.
+
+        This came down to a liveness bug and a register allocation bug.
+
+        The liveness bug was that the code that determined whether we should go around the fixpoint
+        assumed that BitVector::quickSet() would return true if the bit changed state from false to
+        true. That's not how it works. It returns the old value of the bit, so it will return false
+        if the bit changed from false to true. Since there is already a lot of code that relies on
+        this behavior, I fixed Liveness instead of changing BitVector.
+
+        The register allocation bug was that we weren't guarding some checks of tmp()'s with checks
+        that the Arg isTmp().
+
+        The liveness took a long time to track down, and I needed to add a lot of dumping to do it.
+        It's now possible to dump more of the liveness states, including liveAtHead. I found this
+        extremely helpful, so I removed the code that cleared liveAtHead.
+
+        * b3/air/AirIteratedRegisterCoalescing.cpp:
+        * b3/air/AirLiveness.h:
+        (JSC::B3::Air::AbstractLiveness::AbstractLiveness):
+        (JSC::B3::Air::AbstractLiveness::Iterable::Iterable):
+        (JSC::B3::Air::AbstractLiveness::Iterable::iterator::iterator):
+        (JSC::B3::Air::AbstractLiveness::Iterable::iterator::operator*):
+        (JSC::B3::Air::AbstractLiveness::Iterable::iterator::operator++):
+        (JSC::B3::Air::AbstractLiveness::Iterable::iterator::operator==):
+        (JSC::B3::Air::AbstractLiveness::Iterable::iterator::operator!=):
+        (JSC::B3::Air::AbstractLiveness::Iterable::begin):
+        (JSC::B3::Air::AbstractLiveness::Iterable::end):
+        (JSC::B3::Air::AbstractLiveness::liveAtHead):
+        (JSC::B3::Air::AbstractLiveness::liveAtTail):
+        * b3/air/AirStackSlot.h:
+        (WTF::printInternal):
+        * ftl/FTLOSRExitCompiler.cpp:
+        (JSC::FTL::compileFTLOSRExit):
+
 2015-12-22  Saam barati  <sbarati@apple.com>
 
         Cloop build fix after https://bugs.webkit.org/show_bug.cgi?id=152511.
index 95b64d7..306811c 100644 (file)
@@ -1190,7 +1190,8 @@ private:
                 // Move is the canonical way to move data between GPRs.
                 bool forceMove32IfDidSpill = false;
                 bool didSpill = false;
-                if (type == Arg::GP && inst.opcode == Move) {
+                if (type == Arg::GP && inst.opcode == Move
+                    && inst.args[0].isTmp() && inst.args[1].isTmp()) {
                     if (m_tmpWidth.defWidth(inst.args[0].tmp()) <= Arg::Width32
                         || m_tmpWidth.useWidth(inst.args[1].tmp()) <= Arg::Width32)
                         forceMove32IfDidSpill = true;
index 1bbc1af..c0adf6f 100644 (file)
@@ -34,6 +34,7 @@
 #include "B3IndexMap.h"
 #include "B3IndexSet.h"
 #include <wtf/IndexSparseSet.h>
+#include <wtf/ListDump.h>
 
 namespace JSC { namespace B3 { namespace Air {
 
@@ -141,15 +142,13 @@ public:
                     typename Adapter::IndexSet& liveAtTail = m_liveAtTail[predecessor];
                     for (unsigned newValue : m_workset) {
                         if (liveAtTail.add(newValue)) {
-                            if (dirtyBlocks.quickSet(predecessor->index()))
+                            if (!dirtyBlocks.quickSet(predecessor->index()))
                                 changed = true;
                         }
                     }
                 }
             }
         } while (changed);
-
-        m_liveAtHead.clear();
     }
 
     // This calculator has to be run in reverse.
@@ -244,6 +243,74 @@ public:
         BasicBlock* m_block;
     };
 
+    template<typename UnderlyingIterable>
+    class Iterable {
+    public:
+        Iterable(AbstractLiveness& liveness, const UnderlyingIterable& iterable)
+            : m_liveness(liveness)
+            , m_iterable(iterable)
+        {
+        }
+
+        class iterator {
+        public:
+            iterator()
+                : m_liveness(nullptr)
+                , m_iter()
+            {
+            }
+            
+            iterator(AbstractLiveness& liveness, typename UnderlyingIterable::const_iterator iter)
+                : m_liveness(&liveness)
+                , m_iter(iter)
+            {
+            }
+
+            typename Adapter::Thing operator*()
+            {
+                return m_liveness->indexToValue(*m_iter);
+            }
+
+            iterator& operator++()
+            {
+                ++m_iter;
+                return *this;
+            }
+
+            bool operator==(const iterator& other) const
+            {
+                ASSERT(m_liveness == other.m_liveness);
+                return m_iter == other.m_iter;
+            }
+
+            bool operator!=(const iterator& other) const
+            {
+                return !(*this == other);
+            }
+
+        private:
+            AbstractLiveness* m_liveness;
+            typename UnderlyingIterable::const_iterator m_iter;
+        };
+
+        iterator begin() const { return iterator(m_liveness, m_iterable.begin()); }
+        iterator end() const { return iterator(m_liveness, m_iterable.end()); }
+
+    private:
+        AbstractLiveness& m_liveness;
+        const UnderlyingIterable& m_iterable;
+    };
+
+    Iterable<Vector<unsigned>> liveAtHead(BasicBlock* block)
+    {
+        return Iterable<Vector<unsigned>>(*this, m_liveAtHead[block]);
+    }
+
+    Iterable<typename Adapter::IndexSet> liveAtTail(BasicBlock* block)
+    {
+        return Iterable<typename Adapter::IndexSet>(*this, m_liveAtTail[block]);
+    }
+
 private:
     friend class LocalCalc;
     friend struct LocalCalc::Iterable;
index f5703fd..9486730 100644 (file)
@@ -109,6 +109,15 @@ inline DeepStackSlotDump deepDump(const StackSlot* slot)
 
 } } } // namespace JSC::B3::Air
 
+namespace WTF {
+
+inline void printInternal(PrintStream& out, JSC::B3::Air::StackSlot* stackSlot)
+{
+    out.print(*stackSlot);
+}
+
+} // namespace WTF
+
 #endif // ENABLE(B3_JIT)
 
 #endif // AirStackSlot_h
index 242aad4..c388679 100644 (file)
@@ -626,6 +626,9 @@ extern "C" void* compileFTLOSRExit(ExecState* exec, unsigned exitID)
         dataLog("    Will arrive at exit from lazy slow path: ", exit.m_exceptionType == ExceptionType::LazySlowPath, "\n");
 #endif // FTL_USES_B3
         dataLog("    Exit values: ", exit.m_descriptor->m_values, "\n");
+#if FTL_USES_B3
+        dataLog("    Value reps: ", listDump(exit.m_valueReps), "\n");
+#endif // FTL_USES_B3
         if (!exit.m_descriptor->m_materializations.isEmpty()) {
             dataLog("    Materializations:\n");
             for (ExitTimeObjectMaterialization* materialization : exit.m_descriptor->m_materializations)
index 5e05dfa..200d354 100644 (file)
@@ -1,5 +1,20 @@
 2015-12-22  Filip Pizlo  <fpizlo@apple.com>
 
+        FTL B3 should be able to run richards
+        https://bugs.webkit.org/show_bug.cgi?id=152514
+
+        Reviewed by Michael Saboff.
+
+        Change the list dumping helpers to work with a broader set of list kinds.
+
+        * wtf/ListDump.h:
+        (WTF::ListDump::dump):
+        (WTF::MapDump::dump):
+        (WTF::sortedMapDump):
+        (WTF::ListDumpInContext::dump):
+
+2015-12-22  Filip Pizlo  <fpizlo@apple.com>
+
         FTL B3 does not logicalNot correctly
         https://bugs.webkit.org/show_bug.cgi?id=152512
 
index a4317c8..84c6df1 100644 (file)
@@ -43,7 +43,7 @@ public:
     
     void dump(PrintStream& out) const
     {
-        for (typename T::const_iterator iter = m_list.begin(); iter != m_list.end(); ++iter)
+        for (auto iter = m_list.begin(); iter != m_list.end(); ++iter)
             out.print(m_comma, *iter);
     }
 
@@ -84,7 +84,7 @@ public:
     
     void dump(PrintStream& out) const
     {
-        for (typename T::const_iterator iter = m_map.begin(); iter != m_map.end(); ++iter)
+        for (auto iter = m_map.begin(); iter != m_map.end(); ++iter)
             out.print(m_comma, iter->key, m_arrow, iter->value);
     }
     
@@ -135,7 +135,7 @@ template<typename T, typename Comparator>
 CString sortedMapDump(const T& map, const Comparator& comparator, const char* arrow = "=>", const char* comma = ", ")
 {
     Vector<typename T::KeyType> keys;
-    for (typename T::const_iterator iter = map.begin(); iter != map.end(); ++iter)
+    for (auto iter = map.begin(); iter != map.end(); ++iter)
         keys.append(iter->key);
     std::sort(keys.begin(), keys.end(), comparator);
     StringPrintStream out;
@@ -157,7 +157,7 @@ public:
     
     void dump(PrintStream& out) const
     {
-        for (typename T::const_iterator iter = m_list.begin(); iter != m_list.end(); ++iter)
+        for (auto iter = m_list.begin(); iter != m_list.end(); ++iter)
             out.print(m_comma, inContext(*iter, m_context));
     }