Changeset 250999 in webkit for trunk/Source/JavaScriptCore/b3

Timestamp:

Oct 10, 2019, 7:23:37 PM (6 years ago)

Author:

[email protected]

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:

Location:

trunk/Source/JavaScriptCore/b3

Files:

• B3Bank.h (modified) (2 diffs)
• air/AirAllocateRegistersAndStackAndGenerateCode.cpp (modified) (4 diffs)
• air/AirAllocateRegistersAndStackAndGenerateCode.h (modified) (1 diff)

trunk/Source/JavaScriptCore/b3/B3Bank.h

@@ -29 +29 @@
 
 #include "B3Type.h"
+#include "Reg.h"
 
 namespace JSC { namespace B3 {
@@ -64 +65 @@
 }
 
+inline Bank bankForReg(Reg reg)
+{
+    return reg.isGPR() ? GP : FP;
+}
+
 } } // namespace JSC::B3
 

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

@@ -46 +46 @@
 { }
 
+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)
 {
@@ -417 +445 @@
         bool isReplayingSameInst = false;
         for (size_t instIndex = 0; instIndex < block->size(); ++instIndex) {
+            checkConsistency();
+
             if (instIndex && !isReplayingSameInst)
                 startLabel = m_jit->labelIgnoringWatchpoints();
@@ -445 +475 @@
                     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.
@@ -463 +497 @@
                 return false;
             })();
+            checkConsistency();
 
             inst.forEachArg([&] (Arg& arg, Arg::Role role, Bank, Width) {

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

@@ -65 +65 @@
     bool isDisallowedRegister(Reg);
 
+    void checkConsistency();
+
     Code& m_code;
     CCallHelpers* m_jit { nullptr };