Title: [250999] trunk/Source/_javascript_Core
Revision
250999
Author
[email protected]
Date
2019-10-10 19:23:37 -0700 (Thu, 10 Oct 2019)

Log Message

GenerateAndAllocateRegisters can trivially elide self moves at end of liveness
https://bugs.webkit.org/show_bug.cgi?id=202833

Reviewed by Saam Barati.

This also fixes a bug where if a tmp is moved to itself at the end of its lifetime
we would mess up the accounting for the tmp.

In order to catch these bugs earlier during generation I added a
checkConsistency function that if a tmp is in a reg that reg is
not available and that reg thinks the tmp is also allocated in it.

* b3/B3Bank.h:
(JSC::B3::bankForReg):
* b3/air/AirAllocateRegistersAndStackAndGenerateCode.cpp:
(JSC::B3::Air::GenerateAndAllocateRegisters::checkConsistency):
(JSC::B3::Air::GenerateAndAllocateRegisters::generate):
* b3/air/AirAllocateRegistersAndStackAndGenerateCode.h:

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (250998 => 250999)


--- trunk/Source/_javascript_Core/ChangeLog	2019-10-11 01:50:31 UTC (rev 250998)
+++ trunk/Source/_javascript_Core/ChangeLog	2019-10-11 02:23:37 UTC (rev 250999)
@@ -1,3 +1,24 @@
+2019-10-10  Keith Miller  <[email protected]>
+
+        GenerateAndAllocateRegisters can trivially elide self moves at end of liveness
+        https://bugs.webkit.org/show_bug.cgi?id=202833
+
+        Reviewed by Saam Barati.
+
+        This also fixes a bug where if a tmp is moved to itself at the end of its lifetime
+        we would mess up the accounting for the tmp.
+
+        In order to catch these bugs earlier during generation I added a
+        checkConsistency function that if a tmp is in a reg that reg is
+        not available and that reg thinks the tmp is also allocated in it.
+
+        * b3/B3Bank.h:
+        (JSC::B3::bankForReg):
+        * b3/air/AirAllocateRegistersAndStackAndGenerateCode.cpp:
+        (JSC::B3::Air::GenerateAndAllocateRegisters::checkConsistency):
+        (JSC::B3::Air::GenerateAndAllocateRegisters::generate):
+        * b3/air/AirAllocateRegistersAndStackAndGenerateCode.h:
+
 2019-10-10  Yury Semikhatsky  <[email protected]>
 
         Web Inspector: use more C++ keywords for defining agents

Modified: trunk/Source/_javascript_Core/b3/B3Bank.h (250998 => 250999)


--- trunk/Source/_javascript_Core/b3/B3Bank.h	2019-10-11 01:50:31 UTC (rev 250998)
+++ trunk/Source/_javascript_Core/b3/B3Bank.h	2019-10-11 02:23:37 UTC (rev 250999)
@@ -28,6 +28,7 @@
 #if ENABLE(B3_JIT)
 
 #include "B3Type.h"
+#include "Reg.h"
 
 namespace JSC { namespace B3 {
 
@@ -63,6 +64,11 @@
     return GP;
 }
 
+inline Bank bankForReg(Reg reg)
+{
+    return reg.isGPR() ? GP : FP;
+}
+
 } } // namespace JSC::B3
 
 namespace WTF {

Modified: trunk/Source/_javascript_Core/b3/air/AirAllocateRegistersAndStackAndGenerateCode.cpp (250998 => 250999)


--- trunk/Source/_javascript_Core/b3/air/AirAllocateRegistersAndStackAndGenerateCode.cpp	2019-10-11 01:50:31 UTC (rev 250998)
+++ trunk/Source/_javascript_Core/b3/air/AirAllocateRegistersAndStackAndGenerateCode.cpp	2019-10-11 02:23:37 UTC (rev 250999)
@@ -45,6 +45,34 @@
     , m_map(code)
 { }
 
+ALWAYS_INLINE void GenerateAndAllocateRegisters::checkConsistency()
+{
+#if !ASSERT_DISABLED
+    m_code.forEachTmp([&] (Tmp tmp) {
+        Reg reg = m_map[tmp].reg;
+        if (!reg)
+            return;
+
+        ASSERT(!m_availableRegs[tmp.bank()].contains(reg));
+        ASSERT(m_currentAllocation->at(reg) == tmp);
+    });
+
+    for (Reg reg : RegisterSet::allRegisters()) {
+        if (isDisallowedRegister(reg))
+            continue;
+
+        Tmp tmp = m_currentAllocation->at(reg);
+        if (!tmp) {
+            ASSERT(m_availableRegs[bankForReg(reg)].contains(reg));
+            continue;
+        }
+
+        ASSERT(!m_availableRegs[tmp.bank()].contains(reg));
+        ASSERT(m_map[tmp].reg == reg);
+    }
+#endif
+}
+
 void GenerateAndAllocateRegisters::buildLiveRanges(UnifiedTmpLiveness& liveness)
 {
     m_liveRangeEnd = TmpMap<size_t>(m_code, 0);
@@ -416,6 +444,8 @@
 
         bool isReplayingSameInst = false;
         for (size_t instIndex = 0; instIndex < block->size(); ++instIndex) {
+            checkConsistency();
+
             if (instIndex && !isReplayingSameInst)
                 startLabel = m_jit->labelIgnoringWatchpoints();
 
@@ -444,6 +474,10 @@
                 if (source.isReg() || m_liveRangeEnd[source.tmp()] != m_globalInstIndex)
                     return true;
 
+                // If we are doing a self move at the end of the temps liveness we can trivially elide the move.
+                if (source == dest)
+                    return false;
+
                 Reg sourceReg = m_map[source.tmp()].reg;
                 // If the value is not already materialized into a register we may still move it into one so let the normal generation code run.
                 if (!sourceReg)
@@ -462,6 +496,7 @@
                 m_map[source.tmp()].reg = Reg();
                 return false;
             })();
+            checkConsistency();
 
             inst.forEachArg([&] (Arg& arg, Arg::Role role, Bank, Width) {
                 if (!arg.isTmp())

Modified: trunk/Source/_javascript_Core/b3/air/AirAllocateRegistersAndStackAndGenerateCode.h (250998 => 250999)


--- trunk/Source/_javascript_Core/b3/air/AirAllocateRegistersAndStackAndGenerateCode.h	2019-10-11 01:50:31 UTC (rev 250998)
+++ trunk/Source/_javascript_Core/b3/air/AirAllocateRegistersAndStackAndGenerateCode.h	2019-10-11 02:23:37 UTC (rev 250999)
@@ -64,6 +64,8 @@
     void buildLiveRanges(UnifiedTmpLiveness&);
     bool isDisallowedRegister(Reg);
 
+    void checkConsistency();
+
     Code& m_code;
     CCallHelpers* m_jit { nullptr };
 
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to