Modified: trunk/Source/_javascript_Core/ChangeLog (206999 => 207000)
--- trunk/Source/_javascript_Core/ChangeLog 2016-10-10 16:23:57 UTC (rev 206999)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-10-10 16:45:15 UTC (rev 207000)
@@ -1,3 +1,23 @@
+2016-10-08 Filip Pizlo <fpi...@apple.com>
+
+ B3 should know about mutable pinned registers
+ https://bugs.webkit.org/show_bug.cgi?id=163172
+
+ Reviewed by Keith Miller.
+
+ If we have mutable pinned registers then we need to know which operations mutate them. At
+ first I considered making this into a heap range thing, but I think that this would be very
+ confusing. Also, in the future, we might want to make Effects track register sets of
+ clobbered registers (see bug 163173).
+
+ * b3/B3Effects.cpp:
+ (JSC::B3::Effects::interferes):
+ (JSC::B3::Effects::operator==):
+ (JSC::B3::Effects::dump):
+ * b3/B3Effects.h:
+ (JSC::B3::Effects::forCall):
+ (JSC::B3::Effects::mustExecute):
+
2016-10-08 Saam Barati <sbar...@apple.com>
HasIndexedProperty clobberize rule is wrong for Array::ForceOSRExit
Modified: trunk/Source/_javascript_Core/b3/B3Effects.cpp (206999 => 207000)
--- trunk/Source/_javascript_Core/b3/B3Effects.cpp 2016-10-10 16:23:57 UTC (rev 206999)
+++ trunk/Source/_javascript_Core/b3/B3Effects.cpp 2016-10-10 16:45:15 UTC (rev 207000)
@@ -44,7 +44,7 @@
{
if (!terminal.terminal)
return false;
- return other.terminal || other.controlDependent || other.writesLocalState || other.writes;
+ return other.terminal || other.controlDependent || other.writesLocalState || other.writes || other.writesPinned;
}
bool interferesWithExitSideways(const Effects& exitsSideways, const Effects& other)
@@ -51,7 +51,7 @@
{
if (!exitsSideways.exitsSideways)
return false;
- return other.controlDependent || other.writes;
+ return other.controlDependent || other.writes || other.writesPinned;
}
bool interferesWithWritesLocalState(const Effects& writesLocalState, const Effects& other)
@@ -61,17 +61,26 @@
return other.writesLocalState || other.readsLocalState;
}
+bool interferesWithWritesPinned(const Effects& writesPinned, const Effects& other)
+{
+ if (!writesPinned.writesPinned)
+ return false;
+ return other.writesPinned || other.readsPinned;
+}
+
} // anonymous namespace
bool Effects::interferes(const Effects& other) const
{
- if (interferesWithTerminal(*this, other) || interferesWithTerminal(other, *this))
- return true;
- if (interferesWithExitSideways(*this, other) || interferesWithExitSideways(other, *this))
- return true;
- if (interferesWithWritesLocalState(*this, other) || interferesWithWritesLocalState(other, *this))
- return true;
- return writes.overlaps(other.writes)
+ return interferesWithTerminal(*this, other)
+ || interferesWithTerminal(other, *this)
+ || interferesWithExitSideways(*this, other)
+ || interferesWithExitSideways(other, *this)
+ || interferesWithWritesLocalState(*this, other)
+ || interferesWithWritesLocalState(other, *this)
+ || interferesWithWritesPinned(*this, other)
+ || interferesWithWritesPinned(other, *this)
+ || writes.overlaps(other.writes)
|| writes.overlaps(other.reads)
|| reads.overlaps(other.writes);
}
@@ -83,6 +92,8 @@
&& controlDependent == other.controlDependent
&& writesLocalState == other.writesLocalState
&& readsLocalState == other.readsLocalState
+ && writesPinned == other.writesPinned
+ && readsPinned == other.readsPinned
&& writes == other.writes
&& reads == other.reads;
}
@@ -105,6 +116,10 @@
out.print(comma, "WritesLocalState");
if (readsLocalState)
out.print(comma, "ReadsLocalState");
+ if (writesPinned)
+ out.print(comma, "WritesPinned");
+ if (readsPinned)
+ out.print(comma, "ReadsPinned");
if (writes)
out.print(comma, "Writes:", writes);
if (reads)
Modified: trunk/Source/_javascript_Core/b3/B3Effects.h (206999 => 207000)
--- trunk/Source/_javascript_Core/b3/B3Effects.h 2016-10-10 16:23:57 UTC (rev 206999)
+++ trunk/Source/_javascript_Core/b3/B3Effects.h 2016-10-10 16:45:15 UTC (rev 207000)
@@ -60,9 +60,21 @@
// True if this reads from the local state. This is only used for Phi and Get.
bool readsLocalState { false };
+ // B3 understands things about pinned registers. Therefore, it needs to know who reads them and
+ // who writes them. We don't track this on a per-register basis because that would be harder and
+ // we don't need it. Note that if you want to construct an immutable pinned register while also
+ // having other pinned registers that are mutable, then you can use ArgumentReg. Also note that
+ // nobody will stop you from making this get out-of-sync with your clobbered register sets in
+ // Patchpoint. It's recommended that you err on the side of being conservative.
+ // FIXME: Explore making these be RegisterSets. That's mainly hard because it would be awkward to
+ // reconcile with StackmapValue's support for clobbered regs.
+ // https://bugs.webkit.org/show_bug.cgi?id=163173
+ bool readsPinned { false };
+ bool writesPinned { false };
+
HeapRange writes;
HeapRange reads;
-
+
static Effects none()
{
return Effects();
@@ -75,6 +87,8 @@
result.controlDependent = true;
result.writes = HeapRange::top();
result.reads = HeapRange::top();
+ result.readsPinned = true;
+ result.writesPinned = true;
return result;
}
@@ -89,7 +103,7 @@
bool mustExecute() const
{
- return terminal || exitsSideways || writesLocalState || writes;
+ return terminal || exitsSideways || writesLocalState || writes || writesPinned;
}
// Returns true if reordering instructions with these respective effects would change program
