Title: [195296] trunk/Source/_javascript_Core
Revision
195296
Author
fpi...@apple.com
Date
2016-01-19 10:34:19 -0800 (Tue, 19 Jan 2016)

Log Message

Fix Air shuffling assertions
https://bugs.webkit.org/show_bug.cgi?id=153213

Reviewed by Saam Barati.

Fixes some assertions that I was seeing running JSC tests. Adds a new Air test.

* assembler/MacroAssemblerX86Common.h:
(JSC::MacroAssemblerX86Common::store8):
(JSC::MacroAssemblerX86Common::getUnusedRegister):
* b3/air/AirEmitShuffle.cpp:
(JSC::B3::Air::emitShuffle):
* b3/air/AirLowerAfterRegAlloc.cpp:
(JSC::B3::Air::lowerAfterRegAlloc):
* b3/air/testair.cpp:
(JSC::B3::Air::testShuffleRotateWithFringe):
(JSC::B3::Air::testShuffleRotateWithFringeInWeirdOrder):
(JSC::B3::Air::testShuffleRotateWithLongFringe):
(JSC::B3::Air::run):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (195295 => 195296)


--- trunk/Source/_javascript_Core/ChangeLog	2016-01-19 18:21:26 UTC (rev 195295)
+++ trunk/Source/_javascript_Core/ChangeLog	2016-01-19 18:34:19 UTC (rev 195296)
@@ -1,3 +1,25 @@
+2016-01-18  Filip Pizlo  <fpi...@apple.com>
+
+        Fix Air shuffling assertions
+        https://bugs.webkit.org/show_bug.cgi?id=153213
+
+        Reviewed by Saam Barati.
+
+        Fixes some assertions that I was seeing running JSC tests. Adds a new Air test.
+
+        * assembler/MacroAssemblerX86Common.h:
+        (JSC::MacroAssemblerX86Common::store8):
+        (JSC::MacroAssemblerX86Common::getUnusedRegister):
+        * b3/air/AirEmitShuffle.cpp:
+        (JSC::B3::Air::emitShuffle):
+        * b3/air/AirLowerAfterRegAlloc.cpp:
+        (JSC::B3::Air::lowerAfterRegAlloc):
+        * b3/air/testair.cpp:
+        (JSC::B3::Air::testShuffleRotateWithFringe):
+        (JSC::B3::Air::testShuffleRotateWithFringeInWeirdOrder):
+        (JSC::B3::Air::testShuffleRotateWithLongFringe):
+        (JSC::B3::Air::run):
+
 2016-01-19  Konstantin Tokarev  <annu...@yandex.ru>
 
         [mips] Logical instructions allow immediates in range 0..0xffff, not 0x7fff

Modified: trunk/Source/_javascript_Core/assembler/MacroAssemblerX86Common.h (195295 => 195296)


--- trunk/Source/_javascript_Core/assembler/MacroAssemblerX86Common.h	2016-01-19 18:21:26 UTC (rev 195295)
+++ trunk/Source/_javascript_Core/assembler/MacroAssemblerX86Common.h	2016-01-19 18:34:19 UTC (rev 195296)
@@ -733,14 +733,12 @@
 
     void store8(TrustedImm32 imm, Address address)
     {
-        ASSERT(-128 <= imm.m_value && imm.m_value < 128);
-        m_assembler.movb_i8m(imm.m_value, address.offset, address.base);
+        m_assembler.movb_i8m(static_cast<int8_t>(imm.m_value), address.offset, address.base);
     }
 
     void store8(TrustedImm32 imm, BaseIndex address)
     {
-        ASSERT(-128 <= imm.m_value && imm.m_value < 128);
-        m_assembler.movb_i8m(imm.m_value, address.offset, address.base, address.index, address.scale);
+        m_assembler.movb_i8m(static_cast<int8_t>(imm.m_value), address.offset, address.base, address.index, address.scale);
     }
 
     static ALWAYS_INLINE RegisterID getUnusedRegister(BaseIndex address)

Modified: trunk/Source/_javascript_Core/b3/air/AirEmitShuffle.cpp (195295 => 195296)


--- trunk/Source/_javascript_Core/b3/air/AirEmitShuffle.cpp	2016-01-19 18:21:26 UTC (rev 195295)
+++ trunk/Source/_javascript_Core/b3/air/AirEmitShuffle.cpp	2016-01-19 18:34:19 UTC (rev 195296)
@@ -73,6 +73,12 @@
 Vector<Inst> emitShuffle(
     Vector<ShufflePair> pairs, std::array<Arg, 2> scratches, Arg::Type type, Value* origin)
 {
+    if (verbose) {
+        dataLog(
+            "Dealing with pairs: ", listDump(pairs), " and scratches ", scratches[0], ", ",
+            scratches[1], "\n");
+    }
+    
     pairs.removeAllMatching(
         [&] (const ShufflePair& pair) -> bool {
             return pair.src() == pair.dst();
@@ -170,10 +176,18 @@
                 if (verbose)
                     dataLog("It's a rotate.\n");
                 Rotate rotate;
+
+                // The common case is that the rotate does not have fringe. The only way to
+                // check for this is to examine the whole rotate.
+                bool ok;
+                if (currentPairs.last().dst() == originalSrc) {
+                    ok = true;
+                    for (unsigned i = currentPairs.size() - 1; i--;)
+                        ok &= currentPairs[i].dst() == currentPairs[i + 1].src();
+                } else
+                    ok = false;
                 
-                // The common case is that the rotate does not have fringe. When this happens, the
-                // last destination is the first source.
-                if (currentPairs.last().dst() == originalSrc)
+                if (ok)
                     rotate.loop = WTFMove(currentPairs);
                 else {
                     // This is the slow path. The rotate has fringe.

Modified: trunk/Source/_javascript_Core/b3/air/AirLowerAfterRegAlloc.cpp (195295 => 195296)


--- trunk/Source/_javascript_Core/b3/air/AirLowerAfterRegAlloc.cpp	2016-01-19 18:21:26 UTC (rev 195295)
+++ trunk/Source/_javascript_Core/b3/air/AirLowerAfterRegAlloc.cpp	2016-01-19 18:34:19 UTC (rev 195296)
@@ -211,6 +211,11 @@
                     pairs.append(pair);
                 }
 
+                // For finding scratch registers, we need to account for the possibility that
+                // the result is dead.
+                if (originalResult.isReg())
+                    liveRegs.set(originalResult.reg());
+
                 gpScratch = getScratches(liveRegs, Arg::GP);
                 fpScratch = getScratches(liveRegs, Arg::FP);
                 

Modified: trunk/Source/_javascript_Core/b3/air/testair.cpp (195295 => 195296)


--- trunk/Source/_javascript_Core/b3/air/testair.cpp	2016-01-19 18:21:26 UTC (rev 195295)
+++ trunk/Source/_javascript_Core/b3/air/testair.cpp	2016-01-19 18:34:19 UTC (rev 195296)
@@ -676,6 +676,51 @@
     CHECK(things[5] == 3);
 }
 
+void testShuffleRotateWithFringeInWeirdOrder()
+{
+    B3::Procedure proc;
+    Code& code = proc.code();
+
+    BasicBlock* root = code.addBlock();
+    loadConstant(root, 1, Tmp(GPRInfo::regT0));
+    loadConstant(root, 2, Tmp(GPRInfo::regT1));
+    loadConstant(root, 3, Tmp(GPRInfo::regT2));
+    loadConstant(root, 4, Tmp(GPRInfo::regT3));
+    loadConstant(root, 5, Tmp(GPRInfo::regT4));
+    loadConstant(root, 6, Tmp(GPRInfo::regT5));
+    root->append(
+        Shuffle, nullptr,
+        Tmp(GPRInfo::regT0), Tmp(GPRInfo::regT3), Arg::widthArg(Arg::Width32),
+        Tmp(GPRInfo::regT0), Tmp(GPRInfo::regT1), Arg::widthArg(Arg::Width32),
+        Tmp(GPRInfo::regT1), Tmp(GPRInfo::regT4), Arg::widthArg(Arg::Width32),
+        Tmp(GPRInfo::regT2), Tmp(GPRInfo::regT0), Arg::widthArg(Arg::Width32),
+        Tmp(GPRInfo::regT2), Tmp(GPRInfo::regT5), Arg::widthArg(Arg::Width32),
+        Tmp(GPRInfo::regT1), Tmp(GPRInfo::regT2), Arg::widthArg(Arg::Width32));
+
+    int32_t things[6];
+    Tmp base = code.newTmp(Arg::GP);
+    root->append(Move, nullptr, Arg::imm64(bitwise_cast<intptr_t>(&things)), base);
+    root->append(Move32, nullptr, Tmp(GPRInfo::regT0), Arg::addr(base, 0 * sizeof(int32_t)));
+    root->append(Move32, nullptr, Tmp(GPRInfo::regT1), Arg::addr(base, 1 * sizeof(int32_t)));
+    root->append(Move32, nullptr, Tmp(GPRInfo::regT2), Arg::addr(base, 2 * sizeof(int32_t)));
+    root->append(Move32, nullptr, Tmp(GPRInfo::regT3), Arg::addr(base, 3 * sizeof(int32_t)));
+    root->append(Move32, nullptr, Tmp(GPRInfo::regT4), Arg::addr(base, 4 * sizeof(int32_t)));
+    root->append(Move32, nullptr, Tmp(GPRInfo::regT5), Arg::addr(base, 5 * sizeof(int32_t)));
+    root->append(Move, nullptr, Arg::imm(0), Tmp(GPRInfo::returnValueGPR));
+    root->append(Ret32, nullptr, Tmp(GPRInfo::returnValueGPR));
+
+    memset(things, 0, sizeof(things));
+    
+    CHECK(!compileAndRun<int>(proc));
+
+    CHECK(things[0] == 3);
+    CHECK(things[1] == 1);
+    CHECK(things[2] == 2);
+    CHECK(things[3] == 1);
+    CHECK(things[4] == 2);
+    CHECK(things[5] == 3);
+}
+
 void testShuffleRotateWithLongFringe()
 {
     B3::Procedure proc;
@@ -1625,6 +1670,7 @@
     RUN(testShuffleTreeShiftOtherBackward());
     RUN(testShuffleMultipleShifts());
     RUN(testShuffleRotateWithFringe());
+    RUN(testShuffleRotateWithFringeInWeirdOrder());
     RUN(testShuffleRotateWithLongFringe());
     RUN(testShuffleMultipleRotates());
     RUN(testShuffleShiftAndRotate());

Modified: trunk/Source/_javascript_Core/dfg/DFGCommon.h (195295 => 195296)


--- trunk/Source/_javascript_Core/dfg/DFGCommon.h	2016-01-19 18:21:26 UTC (rev 195295)
+++ trunk/Source/_javascript_Core/dfg/DFGCommon.h	2016-01-19 18:34:19 UTC (rev 195296)
@@ -38,7 +38,7 @@
 // We are in the middle of an experimental transition from LLVM to B3 as the backend for the FTL. We don't
 // yet know how it will turn out. For now, this flag will control whether FTL uses B3. Remember to set this
 // to 0 before committing!
-#define FTL_USES_B3 0
+#define FTL_USES_B3 1
 
 struct Node;
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to