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:
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@250999 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index 2d6d32e..836845a 100644
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,24 @@
+2019-10-10 Keith Miller <keith_miller@apple.com>
+
+ 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 <yurys@chromium.org>
Web Inspector: use more C++ keywords for defining agents
diff --git a/Source/JavaScriptCore/b3/B3Bank.h b/Source/JavaScriptCore/b3/B3Bank.h
index 1e93bad..5bebf26 100644
--- a/Source/JavaScriptCore/b3/B3Bank.h
+++ b/Source/JavaScriptCore/b3/B3Bank.h
@@ -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 {
diff --git a/Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackAndGenerateCode.cpp b/Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackAndGenerateCode.cpp
index 3f9c79c..ea2c7cc 100644
--- a/Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackAndGenerateCode.cpp
+++ b/Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackAndGenerateCode.cpp
@@ -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())
diff --git a/Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackAndGenerateCode.h b/Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackAndGenerateCode.h
index df7a3ff..ac3cebf 100644
--- a/Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackAndGenerateCode.h
+++ b/Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackAndGenerateCode.h
@@ -64,6 +64,8 @@
void buildLiveRanges(UnifiedTmpLiveness&);
bool isDisallowedRegister(Reg);
+ void checkConsistency();
+
Code& m_code;
CCallHelpers* m_jit { nullptr };
