Changeset 182643 in webkit

Timestamp:

Apr 10, 2015, 5:00:38 PM (10 years ago)

Author:

msaboff@apple.com

Message:

REGRESSION (182567): regress/script-tests/sorting-benchmark.js fails on 32 bit dfg-eager tests
​https://bugs.webkit.org/show_bug.cgi?id=143582

Reviewed by Mark Lam.

For 32 bit builds, we favor spilling unboxed values. The ASSERT at the root of this bug doesn't
fire for 64 bit builds, because we spill an "Other" value as a full JS value (DataFormatJS).
For 32 bit builds however, if we are able, we spill Other values as JSCell* (DataFormatCell).
The fix is to add a check in fillSpeculateInt32Internal() before the ASSERT that always OSR exits
if the spillFormat is DataFormatCell. Had we spilled in DataFormatJS and the value was a JSCell*,
we would still OSR exit after the speculation check.

• dfg/DFGFixupPhase.cpp:

(JSC::DFG::FixupPhase::fixupNode): Fixed an error in a comment while debugging.

• dfg/DFGSpeculativeJIT32_64.cpp:

(JSC::DFG::SpeculativeJIT::fillSpeculateInt32Internal):

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• dfg/DFGFixupPhase.cpp (modified) (1 diff)
• dfg/DFGSpeculativeJIT32_64.cpp (modified) (1 diff)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2015-04-10  Michael Saboff  <msaboff@apple.com>
+
+        REGRESSION (182567): regress/script-tests/sorting-benchmark.js fails on 32 bit dfg-eager tests
+        https://bugs.webkit.org/show_bug.cgi?id=143582
+
+        Reviewed by Mark Lam.
+
+        For 32 bit builds, we favor spilling unboxed values.  The ASSERT at the root of this bug doesn't
+        fire for 64 bit builds, because we spill an "Other" value as a full JS value (DataFormatJS).
+        For 32 bit builds however, if we are able, we spill Other values as JSCell* (DataFormatCell).
+        The fix is to add a check in fillSpeculateInt32Internal() before the ASSERT that always OSR exits
+        if the spillFormat is DataFormatCell.  Had we spilled in DataFormatJS and the value was a JSCell*,
+        we would still OSR exit after the speculation check.
+
+        * dfg/DFGFixupPhase.cpp:
+        (JSC::DFG::FixupPhase::fixupNode): Fixed an error in a comment while debugging.
+        * dfg/DFGSpeculativeJIT32_64.cpp:
+        (JSC::DFG::SpeculativeJIT::fillSpeculateInt32Internal):
+
 2015-04-10  Milan Crha  <mcrha@redhat.com>
 

trunk/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp

@@ -96 +96 @@
         switch (op) {
         case SetLocal: {
-            // This gets handled by fixupSetLocalsInBlock().
+            // This gets handled by fixupGetAndSetLocalsInBlock().
             return;
         }

trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp

@@ -869 +869 @@
 
         DataFormat spillFormat = info.spillFormat();
+
+        if (spillFormat == DataFormatCell) {
+            terminateSpeculativeExecution(BadType, JSValueRegs(), edge);
+            returnFormat = DataFormatInt32;
+            return allocate();
+        }
+
         ASSERT_UNUSED(spillFormat, (spillFormat & DataFormatJS) || spillFormat == DataFormatInt32);