Changeset 194382 in webkit

Timestamp:

Dec 22, 2015, 5:50:19 PM (10 years ago)

Author:

fpizlo@apple.com

Message:

FTL B3 should be able to run richards
​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):

Location:

trunk/Source

Files:

• JavaScriptCore/ChangeLog (modified) (1 diff)
• JavaScriptCore/b3/air/AirIteratedRegisterCoalescing.cpp (modified) (1 diff)
• JavaScriptCore/b3/air/AirLiveness.h (modified) (4 diffs)
• JavaScriptCore/b3/air/AirStackSlot.h (modified) (1 diff)
• JavaScriptCore/ftl/FTLOSRExitCompiler.cpp (modified) (1 diff)
• WTF/ChangeLog (modified) (1 diff)
• WTF/wtf/ListDump.h (modified) (4 diffs)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/JavaScriptCore/b3/air/AirIteratedRegisterCoalescing.cpp

@@ -1191 +1191 @@
                 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)

trunk/Source/JavaScriptCore/b3/air/AirLiveness.h

@@ -35 +35 @@
 #include "B3IndexSet.h"
 #include <wtf/IndexSparseSet.h>
+#include <wtf/ListDump.h>
 
 namespace JSC { namespace B3 { namespace Air {
@@ -142 +143 @@
                     for (unsigned newValue : m_workset) {
                         if (liveAtTail.add(newValue)) {
-                            if (dirtyBlocks.quickSet(predecessor->index()))
+                            if (!dirtyBlocks.quickSet(predecessor->index()))
                                 changed = true;
                         }
@@ -149 +150 @@
             }
         } while (changed);
-
-        m_liveAtHead.clear();
     }
 
@@ -245 +244 @@
     };
 
+    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;

trunk/Source/JavaScriptCore/b3/air/AirStackSlot.h

@@ -110 +110 @@
 } } } // 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)
 

trunk/Source/JavaScriptCore/ftl/FTLOSRExitCompiler.cpp

@@ -627 +627 @@
 #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");

trunk/Source/WTF/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WTF/wtf/ListDump.h

@@ -44 +44 @@
     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);
     }
@@ -85 +85 @@
     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);
     }
@@ -136 +136 @@
 {
     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);
@@ -158 +158 @@
     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));
     }