Changeset 92909 in webkit for trunk/Source/JavaScriptCore/dfg/DFGJITCompiler.cpp

Timestamp:

Aug 11, 2011, 8:41:48 PM (14 years ago)

Author:

[email protected]

Message:

DFG JIT speculation failure code sometimes picks the wrong register
as a scratch register.
​https://bugs.webkit.org/show_bug.cgi?id=66104

Reviewed by Gavin Barraclough.

Hardened the code with more assertions and fixed the bug. Now a
spilled register is only used for scratch if it also isn't being
used for shuffling.

• dfg/DFGJITCompiler.cpp:

(JSC::DFG::ShuffledRegister::handleNonCyclingPermutation):
(JSC::DFG::JITCompiler::jumpFromSpeculativeToNonSpeculative):

File:

• trunk/Source/JavaScriptCore/dfg/DFGJITCompiler.cpp (modified) (7 diffs)

trunk/Source/JavaScriptCore/dfg/DFGJITCompiler.cpp

@@ -327 +327 @@
                 if (scratchFPR1 == InvalidFPRReg)
                     scratchFPR1 = cur->reg.fpr();
-                else
+                else {
+                    ASSERT(scratchFPR1 != cur->reg.fpr());
                     scratchFPR2 = cur->reg.fpr();
+                }
             }
             cur = cur->previous;
@@ -336 +338 @@
             if (scratchFPR1 == InvalidFPRReg)
                 scratchFPR1 = cur->reg.fpr();
-            else
+            else {
+                ASSERT(scratchFPR1 != cur->reg.fpr());
                 scratchFPR2 = cur->reg.fpr();
+            }
         }
     }
@@ -569 +573 @@
         
         // Bail out if this register isn't assigned to anything.
-        if (nodeIndex == NoNode) {
-            scratchGPR = GPRInfo::toRegister(index);
+        if (nodeIndex == NoNode)
             continue;
-        }
         
         // If the non-speculative path also has a register for the nodeIndex that this
@@ -588 +590 @@
             if (!mapIterator->second.findInEntryLocation(entry).isSpilled)
                 continue;
+        } else {
+            // If the non-speculative entry isn't using this register and it does not need
+            // the value in this register to be placed into any other register, then this
+            // register can be used for scratch.
+            if (entry.m_gprInfo[index].nodeIndex == NoNode)
+                scratchGPR = GPRInfo::toRegister(index);
         }
         
@@ -602 +610 @@
             orPtr(GPRInfo::tagTypeNumberRegister, GPRInfo::toRegister(index));
         storePtr(GPRInfo::toRegister(index), addressFor(virtualRegister));
-        
-        scratchGPR = GPRInfo::toRegister(index);
     }
     
@@ -626 +632 @@
             if (!mapIterator->second.findInEntryLocation(entry).isSpilled)
                 continue;
+        } else {
+            // If the non-speculative entry isn't using this register and it does not need
+            // the value in this register to be placed into any other register, then this
+            // register can be used for scratch.
+            if (entry.m_fprInfo[index].nodeIndex == NoNode) {
+                if (scratchFPR1 == InvalidFPRReg)
+                    scratchFPR1 = FPRInfo::toRegister(index);
+                else if (scratchFPR2)
+                    scratchFPR2 = FPRInfo::toRegister(index);
+                ASSERT((scratchFPR1 == InvalidFPRReg && scratchFPR2 == InvalidFPRReg) || (scratchFPR1 != scratchFPR2));
+            }
         }
         
@@ -636 +653 @@
         subPtr(GPRInfo::tagTypeNumberRegister, scratchGPR);
         storePtr(scratchGPR, addressFor(virtualRegister));
-
-        if (scratchFPR1 == InvalidFPRReg)
-            scratchFPR1 = FPRInfo::toRegister(index);
-        else if (scratchFPR2)
-            scratchFPR2 = FPRInfo::toRegister(index);
-        ASSERT((scratchFPR1 == InvalidFPRReg && scratchFPR2 == InvalidFPRReg) || (scratchFPR1 != scratchFPR2));
-    }
+    }
+    
+#if !ASSERT_DISABLED
+    // Assert that we've not assigned a scratch register to something that we're going to shuffle.
+    ASSERT(scratchGPR != InvalidGPRReg);
+    if (scratchGPR != GPRInfo::tagMaskRegister) {
+        ASSERT(!gprs[GPRInfo::toIndex(scratchGPR)].hasTo);
+        ASSERT(!gprs[GPRInfo::toIndex(scratchGPR)].hasFrom);
+    }
+    if (scratchFPR1 != InvalidFPRReg) {
+        ASSERT(scratchFPR1 != scratchFPR2);
+        ASSERT(!fprs[FPRInfo::toIndex(scratchFPR1)].hasTo);
+        ASSERT(!fprs[FPRInfo::toIndex(scratchFPR1)].hasFrom);
+        if (scratchFPR2 != InvalidFPRReg) {
+            ASSERT(!fprs[FPRInfo::toIndex(scratchFPR2)].hasTo);
+            ASSERT(!fprs[FPRInfo::toIndex(scratchFPR2)].hasFrom);
+        }
+    } else
+        ASSERT(scratchFPR2 == InvalidFPRReg);
+#endif
     
     // Part 2: For the set of nodes that are in registers on both paths,