Title: [194382] trunk/Source
Revision
194382
Author
[email protected]
Date
2015-12-22 17:50:19 -0800 (Tue, 22 Dec 2015)

Log Message

FTL B3 should be able to run richards
https://bugs.webkit.org/show_bug.cgi?id=152514

Reviewed by Michael Saboff.

Source/_javascript_Core:

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):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (194381 => 194382)


--- trunk/Source/_javascript_Core/ChangeLog	2015-12-23 01:36:03 UTC (rev 194381)
+++ trunk/Source/_javascript_Core/ChangeLog	2015-12-23 01:50:19 UTC (rev 194382)
@@ -1,3 +1,43 @@
+2015-12-22  Filip Pizlo  <[email protected]>
+
+        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  <[email protected]>
 
         Cloop build fix after https://bugs.webkit.org/show_bug.cgi?id=152511.

Modified: trunk/Source/_javascript_Core/b3/air/AirIteratedRegisterCoalescing.cpp (194381 => 194382)


--- trunk/Source/_javascript_Core/b3/air/AirIteratedRegisterCoalescing.cpp	2015-12-23 01:36:03 UTC (rev 194381)
+++ trunk/Source/_javascript_Core/b3/air/AirIteratedRegisterCoalescing.cpp	2015-12-23 01:50:19 UTC (rev 194382)
@@ -1190,7 +1190,8 @@
                 // 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;

Modified: trunk/Source/_javascript_Core/b3/air/AirLiveness.h (194381 => 194382)


--- trunk/Source/_javascript_Core/b3/air/AirLiveness.h	2015-12-23 01:36:03 UTC (rev 194381)
+++ trunk/Source/_javascript_Core/b3/air/AirLiveness.h	2015-12-23 01:50:19 UTC (rev 194382)
@@ -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 @@
                     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 @@
         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;

Modified: trunk/Source/_javascript_Core/b3/air/AirStackSlot.h (194381 => 194382)


--- trunk/Source/_javascript_Core/b3/air/AirStackSlot.h	2015-12-23 01:36:03 UTC (rev 194381)
+++ trunk/Source/_javascript_Core/b3/air/AirStackSlot.h	2015-12-23 01:50:19 UTC (rev 194382)
@@ -109,6 +109,15 @@
 
 } } } // 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

Modified: trunk/Source/_javascript_Core/ftl/FTLOSRExitCompiler.cpp (194381 => 194382)


--- trunk/Source/_javascript_Core/ftl/FTLOSRExitCompiler.cpp	2015-12-23 01:36:03 UTC (rev 194381)
+++ trunk/Source/_javascript_Core/ftl/FTLOSRExitCompiler.cpp	2015-12-23 01:50:19 UTC (rev 194382)
@@ -626,6 +626,9 @@
         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)

Modified: trunk/Source/WTF/ChangeLog (194381 => 194382)


--- trunk/Source/WTF/ChangeLog	2015-12-23 01:36:03 UTC (rev 194381)
+++ trunk/Source/WTF/ChangeLog	2015-12-23 01:50:19 UTC (rev 194382)
@@ -1,5 +1,20 @@
 2015-12-22  Filip Pizlo  <[email protected]>
 
+        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  <[email protected]>
+
         FTL B3 does not logicalNot correctly
         https://bugs.webkit.org/show_bug.cgi?id=152512
 

Modified: trunk/Source/WTF/wtf/ListDump.h (194381 => 194382)


--- trunk/Source/WTF/wtf/ListDump.h	2015-12-23 01:36:03 UTC (rev 194381)
+++ trunk/Source/WTF/wtf/ListDump.h	2015-12-23 01:50:19 UTC (rev 194382)
@@ -43,7 +43,7 @@
     
     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 @@
     
     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 @@
 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 @@
     
     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));
     }
 
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to