Title: [196035] trunk/Source/_javascript_Core
- Revision
- 196035
- Author
- [email protected]
- Date
- 2016-02-02 15:46:52 -0800 (Tue, 02 Feb 2016)
Log Message
B3 should be able to compile trivial self-loops
https://bugs.webkit.org/show_bug.cgi?id=153802
rdar://problem/24465632
Reviewed by Michael Saboff.
Tail-duplicating a self-loop would mean doing a kind of loop unrolling. It wouldn't be
profitable even if it did work. It turns out that it doesn't work, because we edit the target
block before reading the source block, which breaks if the target and source block are the
same.
This disables tail duplication of self-loops, adds a test, and adds better validation for this
issue.
* b3/B3DuplicateTails.cpp:
* b3/B3Procedure.cpp:
(JSC::B3::Procedure::resetReachability):
* b3/testb3.cpp:
(JSC::B3::testComputeDivisionMagic):
(JSC::B3::testTrivialInfiniteLoop):
(JSC::B3::zero):
(JSC::B3::run):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (196034 => 196035)
--- trunk/Source/_javascript_Core/ChangeLog 2016-02-02 23:42:40 UTC (rev 196034)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-02-02 23:46:52 UTC (rev 196035)
@@ -1,3 +1,28 @@
+2016-02-02 Filip Pizlo <[email protected]>
+
+ B3 should be able to compile trivial self-loops
+ https://bugs.webkit.org/show_bug.cgi?id=153802
+ rdar://problem/24465632
+
+ Reviewed by Michael Saboff.
+
+ Tail-duplicating a self-loop would mean doing a kind of loop unrolling. It wouldn't be
+ profitable even if it did work. It turns out that it doesn't work, because we edit the target
+ block before reading the source block, which breaks if the target and source block are the
+ same.
+
+ This disables tail duplication of self-loops, adds a test, and adds better validation for this
+ issue.
+
+ * b3/B3DuplicateTails.cpp:
+ * b3/B3Procedure.cpp:
+ (JSC::B3::Procedure::resetReachability):
+ * b3/testb3.cpp:
+ (JSC::B3::testComputeDivisionMagic):
+ (JSC::B3::testTrivialInfiniteLoop):
+ (JSC::B3::zero):
+ (JSC::B3::run):
+
2016-02-02 Saam barati <[email protected]>
[ES6] bound functions .name property should be "bound " + the target function's name
Modified: trunk/Source/_javascript_Core/b3/B3DuplicateTails.cpp (196034 => 196035)
--- trunk/Source/_javascript_Core/b3/B3DuplicateTails.cpp 2016-02-02 23:42:40 UTC (rev 196034)
+++ trunk/Source/_javascript_Core/b3/B3DuplicateTails.cpp 2016-02-02 23:46:52 UTC (rev 196035)
@@ -105,6 +105,11 @@
if (!candidates.contains(tail))
continue;
+ // Don't tail duplicate a trivial self-loop, because the code below can't handle block and
+ // tail being the same block.
+ if (block == tail)
+ continue;
+
// We're about to change 'block'. Make sure that nobody duplicates block after this
// point.
candidates.remove(block);
Modified: trunk/Source/_javascript_Core/b3/B3Procedure.cpp (196034 => 196035)
--- trunk/Source/_javascript_Core/b3/B3Procedure.cpp 2016-02-02 23:42:40 UTC (rev 196034)
+++ trunk/Source/_javascript_Core/b3/B3Procedure.cpp 2016-02-02 23:46:52 UTC (rev 196035)
@@ -147,6 +147,31 @@
void Procedure::resetReachability()
{
+ if (shouldValidateIR()) {
+ // Validate the basic properties that we need for resetting reachability. We often reset
+ // reachability before IR validation, so without this mini-validation, you would crash inside
+ // B3::resetReachability() without getting any IR dump.
+
+ BasicBlock* badBlock = nullptr;
+ for (BasicBlock* block : *this) {
+ if (!block->size()) {
+ badBlock = block;
+ break;
+ }
+
+ if (!block->last()->as<ControlValue>()) {
+ badBlock = block;
+ break;
+ }
+ }
+
+ if (badBlock) {
+ dataLog("FATAL: Invalid basic block ", *badBlock, " while running Procedure::resetReachability().\n");
+ dataLog(*this);
+ RELEASE_ASSERT_NOT_REACHED();
+ }
+ }
+
B3::resetReachability(
m_blocks,
[&] (BasicBlock* deleted) {
Modified: trunk/Source/_javascript_Core/b3/testb3.cpp (196034 => 196035)
--- trunk/Source/_javascript_Core/b3/testb3.cpp 2016-02-02 23:42:40 UTC (rev 196034)
+++ trunk/Source/_javascript_Core/b3/testb3.cpp 2016-02-02 23:46:52 UTC (rev 196035)
@@ -10026,6 +10026,17 @@
CHECK(magic.shift == shift);
}
+void testTrivialInfiniteLoop()
+{
+ Procedure proc;
+ BasicBlock* root = proc.addBlock();
+ BasicBlock* loop = proc.addBlock();
+ root->appendNew<ControlValue>(proc, Jump, Origin(), FrequentedBlock(loop));
+ loop->appendNew<ControlValue>(proc, Jump, Origin(), FrequentedBlock(loop));
+
+ compile(proc);
+}
+
// Make sure the compiler does not try to optimize anything out.
NEVER_INLINE double zero()
{
@@ -11427,6 +11438,8 @@
RUN(testComputeDivisionMagic<int32_t>(2, -2147483647, 0));
+ RUN(testTrivialInfiniteLoop());
+
if (tasks.isEmpty())
usage();
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
