Attachment 119109 Details for Bug 74463 – the patch

[patch] the patch

uint32tonumber_patch_1.diff (text/plain), 29.26 KB, created by Filip Pizlo on 2011-12-13 16:53:53 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Filip Pizlo

Created: 2011-12-13 16:53:53 PST

Size: 29.26 KB

patch

obsolete

>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 102720)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,40 @@
>+2011-12-13  Filip Pizlo  <fpizlo@apple.com>
>+
>+        DFG OSR exit for UInt32ToNumber should roll forward, not roll backward
>+        https://bugs.webkit.org/show_bug.cgi?id=74463
>+
>+        Reviewed by NOBODY (OOPS!).
>+        
>+        Implements roll-forward OSR exit for UInt32ToNumber, which requires ValueRecoveries knowing
>+        how to execute the slow path of UInt32ToNumber.
>+
>+        * bytecode/CodeBlock.h:
>+        (JSC::CodeBlock::lastOSRExit):
>+        * bytecode/CodeOrigin.h:
>+        (JSC::CodeOrigin::operator!=):
>+        * bytecode/ValueRecovery.h:
>+        (JSC::ValueRecovery::uint32InGPR):
>+        (JSC::ValueRecovery::gpr):
>+        (JSC::ValueRecovery::dump):
>+        * dfg/DFGAssemblyHelpers.cpp:
>+        * dfg/DFGAssemblyHelpers.h:
>+        * dfg/DFGOSRExit.h:
>+        (JSC::DFG::OSRExit::valueRecoveryForOperand):
>+        * dfg/DFGOSRExitCompiler32_64.cpp:
>+        (JSC::DFG::OSRExitCompiler::compileExit):
>+        * dfg/DFGOSRExitCompiler64.cpp:
>+        (JSC::DFG::OSRExitCompiler::compileExit):
>+        * dfg/DFGSpeculativeJIT.cpp:
>+        (JSC::DFG::SpeculativeJIT::compileUInt32ToNumber):
>+        (JSC::DFG::SpeculativeJIT::compileGetByValOnIntTypedArray):
>+        * dfg/DFGSpeculativeJIT.h:
>+        * dfg/DFGSpeculativeJIT32_64.cpp:
>+        (JSC::DFG::SpeculativeJIT::nonSpeculativeUInt32ToNumber):
>+        (JSC::DFG::SpeculativeJIT::compile):
>+        * dfg/DFGSpeculativeJIT64.cpp:
>+        (JSC::DFG::SpeculativeJIT::nonSpeculativeUInt32ToNumber):
>+        (JSC::DFG::SpeculativeJIT::compile):
>+
> 2011-12-13  Oliver Hunt  <oliver@apple.com>
> 
>         Arguments object doesn't handle mutation of length property correctly
>Index: Source/JavaScriptCore/bytecode/CodeBlock.h
>===================================================================
>--- Source/JavaScriptCore/bytecode/CodeBlock.h	(revision 102701)
>+++ Source/JavaScriptCore/bytecode/CodeBlock.h	(working copy)
>@@ -427,6 +427,11 @@ namespace JSC {
>             m_dfgData->osrExit.append(osrExit);
>         }
>         
>+        DFG::OSRExit& lastOSRExit()
>+        {
>+            return m_dfgData->osrExit.last();
>+        }
>+        
>         void appendSpeculationRecovery(const DFG::SpeculationRecovery& recovery)
>         {
>             createDFGDataIfNecessary();
>Index: Source/JavaScriptCore/bytecode/CodeOrigin.h
>===================================================================
>--- Source/JavaScriptCore/bytecode/CodeOrigin.h	(revision 102701)
>+++ Source/JavaScriptCore/bytecode/CodeOrigin.h	(working copy)
>@@ -73,6 +73,8 @@ struct CodeOrigin {
>     
>     bool operator==(const CodeOrigin& other) const;
>     
>+    bool operator!=(const CodeOrigin& other) const { return !(*this == other); }
>+    
> #ifndef NDEBUG
>     // Get the inline stack. This is slow, and is intended for debugging only.
>     Vector<CodeOrigin> inlineStack() const;
>Index: Source/JavaScriptCore/bytecode/ValueRecovery.h
>===================================================================
>--- Source/JavaScriptCore/bytecode/ValueRecovery.h	(revision 102701)
>+++ Source/JavaScriptCore/bytecode/ValueRecovery.h	(working copy)
>@@ -51,6 +51,7 @@ enum ValueRecoveryTechnique {
>     InPair,
> #endif
>     InFPR,
>+    UInt32InGPR,
>     // It's in the register file, but at a different location.
>     DisplacedInRegisterFile,
>     // It's in the register file, at a different location, and it's unboxed.
>@@ -114,6 +115,14 @@ public:
>         return result;
>     }
>     
>+    static ValueRecovery uint32InGPR(MacroAssembler::RegisterID gpr)
>+    {
>+        ValueRecovery result;
>+        result.m_technique = UInt32InGPR;
>+        result.m_source.gpr = gpr;
>+        return result;
>+    }
>+    
> #if USE(JSVALUE32_64)
>     static ValueRecovery inPair(MacroAssembler::RegisterID tagGPR, MacroAssembler::RegisterID payloadGPR)
>     {
>@@ -182,7 +191,7 @@ public:
>     
>     MacroAssembler::RegisterID gpr() const
>     {
>-        ASSERT(m_technique == InGPR || m_technique == UnboxedInt32InGPR || m_technique == UnboxedBooleanInGPR);
>+        ASSERT(m_technique == InGPR || m_technique == UnboxedInt32InGPR || m_technique == UnboxedBooleanInGPR || m_technique == UInt32InGPR);
>         return m_source.gpr;
>     }
>     
>@@ -243,8 +252,11 @@ public:
>         case UnboxedBooleanInGPR:
>             fprintf(out, "bool(%%r%d)", gpr());
>             break;
>+        case UInt32InGPR:
>+            fprintf(out, "uint32(%%r%d)", gpr());
>+            break;
>         case InFPR:
>-            fprintf(out, "%%r%d", fpr());
>+            fprintf(out, "%%fr%d", fpr());
>             break;
> #if USE(JSVALUE32_64)
>         case InPair:
>Index: Source/JavaScriptCore/dfg/DFGAssemblyHelpers.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGAssemblyHelpers.cpp	(revision 102701)
>+++ Source/JavaScriptCore/dfg/DFGAssemblyHelpers.cpp	(working copy)
>@@ -30,6 +30,8 @@
> 
> namespace JSC { namespace DFG {
> 
>+const double AssemblyHelpers::twoToThe32 = (double)0x100000000ull;
>+
> Vector<BytecodeAndMachineOffset>& AssemblyHelpers::decodedCodeMapFor(CodeBlock* codeBlock)
> {
>     ASSERT(codeBlock == codeBlock->baselineVersion());
>Index: Source/JavaScriptCore/dfg/DFGAssemblyHelpers.h
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGAssemblyHelpers.h	(revision 102701)
>+++ Source/JavaScriptCore/dfg/DFGAssemblyHelpers.h	(working copy)
>@@ -305,6 +305,8 @@ public:
>     }
>     
>     Vector<BytecodeAndMachineOffset>& decodedCodeMapFor(CodeBlock*);
>+    
>+    static const double twoToThe32;
> 
> protected:
>     JSGlobalData* m_globalData;
>Index: Source/JavaScriptCore/dfg/DFGOSRExit.h
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGOSRExit.h	(revision 102701)
>+++ Source/JavaScriptCore/dfg/DFGOSRExit.h	(working copy)
>@@ -107,6 +107,12 @@ struct OSRExit {
>             return m_arguments[index];
>         return m_variables[index - m_arguments.size()];
>     }
>+    ValueRecovery& valueRecoveryForOperand(int operand)
>+    {
>+        if (operandIsArgument(operand))
>+            return m_arguments[operandToArgument(operand)];
>+        return m_variables[operand];
>+    }
>     bool isArgument(int index) const { return index < (int)m_arguments.size(); }
>     bool isVariable(int index) const { return !isArgument(index); }
>     int argumentForIndex(int index) const
>Index: Source/JavaScriptCore/dfg/DFGOSRExitCompiler32_64.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGOSRExitCompiler32_64.cpp	(revision 102701)
>+++ Source/JavaScriptCore/dfg/DFGOSRExitCompiler32_64.cpp	(working copy)
>@@ -124,6 +124,7 @@ void OSRExitCompiler::compileExit(const 
>     bool haveUnboxedInt32InRegisterFile = false;
>     bool haveUnboxedCellInRegisterFile = false;
>     bool haveUnboxedBooleanInRegisterFile = false;
>+    bool haveUInt32s = false;
>     bool haveFPRs = false;
>     bool haveConstants = false;
>     bool haveUndefined = false;
>@@ -147,6 +148,7 @@ void OSRExitCompiler::compileExit(const 
>                 case InGPR:
>                 case UnboxedInt32InGPR:
>                 case UnboxedBooleanInGPR:
>+                case UInt32InGPR:
>                 case InPair:
>                 case InFPR:
>                     if (!poisonedVirtualRegisters[recovery.virtualRegister()]) {
>@@ -160,6 +162,10 @@ void OSRExitCompiler::compileExit(const 
>             }
>             break;
>             
>+        case UInt32InGPR:
>+            haveUInt32s = true;
>+            break;
>+
>         case AlreadyInRegisterFileAsUnboxedInt32:
>             haveUnboxedInt32InRegisterFile = true;
>             break;
>@@ -187,7 +193,8 @@ void OSRExitCompiler::compileExit(const 
>         }
>     }
>     
>-    EncodedJSValue* scratchBuffer = static_cast<EncodedJSValue*>(m_jit.globalData()->scratchBufferForSize(sizeof(EncodedJSValue) * (numberOfPoisonedVirtualRegisters + ((numberOfDisplacedVirtualRegisters * 2) <= GPRInfo::numberOfRegisters ? 0 : numberOfDisplacedVirtualRegisters))));
>+    unsigned scratchBufferLengthBeforeUInt32s = numberOfPoisonedVirtualRegisters + ((numberOfDisplacedVirtualRegisters * 2) <= GPRInfo::numberOfRegisters ? 0 : numberOfDisplacedVirtualRegisters);
>+    EncodedJSValue* scratchBuffer = static_cast<EncodedJSValue*>(m_jit.globalData()->scratchBufferForSize(sizeof(EncodedJSValue) * (scratchBufferLengthBeforeUInt32s + (haveUInt32s ? 2 : 0))));
> 
>     // From here on, the code assumes that it is profitable to maximize the distance
>     // between when something is computed and when it is stored.
>@@ -252,6 +259,49 @@ void OSRExitCompiler::compileExit(const 
>                 m_jit.store32(recovery.payloadGPR(), AssemblyHelpers::payloadFor((VirtualRegister)operand));
>             }
>             break;
>+        case UInt32InGPR: {
>+            EncodedJSValue* myScratch = scratchBuffer + scratchBufferLengthBeforeUInt32s;
>+            
>+            GPRReg addressGPR = GPRInfo::regT0;
>+            if (addressGPR == recovery.gpr())
>+                addressGPR = GPRInfo::regT1;
>+            
>+            m_jit.storePtr(addressGPR, myScratch);
>+            m_jit.move(AssemblyHelpers::TrustedImmPtr(myScratch + 1), addressGPR);
>+            m_jit.storeDouble(FPRInfo::fpRegT0, addressGPR);
>+            
>+            AssemblyHelpers::Jump positive = m_jit.branch32(AssemblyHelpers::GreaterThanOrEqual, recovery.gpr(), AssemblyHelpers::TrustedImm32(0));
>+            
>+            m_jit.convertInt32ToDouble(recovery.gpr(), FPRInfo::fpRegT0);
>+            m_jit.addDouble(AssemblyHelpers::AbsoluteAddress(&AssemblyHelpers::twoToThe32), FPRInfo::fpRegT0);
>+            if (exit.isVariable(index) && poisonedVirtualRegisters[exit.variableForIndex(index)]) {
>+                m_jit.move(AssemblyHelpers::TrustedImmPtr(scratchBuffer + scratchIndex), addressGPR);
>+                m_jit.storeDouble(FPRInfo::fpRegT0, addressGPR);
>+            } else
>+                m_jit.storeDouble(FPRInfo::fpRegT0, AssemblyHelpers::addressFor((VirtualRegister)operand));
>+            
>+            AssemblyHelpers::Jump done = m_jit.jump();
>+            
>+            positive.link(&m_jit);
>+            
>+            if (exit.isVariable(index) && poisonedVirtualRegisters[exit.variableForIndex(index)]) {
>+                m_jit.store32(recovery.gpr(), reinterpret_cast<char*>(scratchBuffer + scratchIndex) + OBJECT_OFFSETOF(EncodedValueDescriptor, asBits.payload));
>+                m_jit.store32(AssemblyHelpers::TrustedImm32(JSValue::Int32Tag), reinterpret_cast<char*>(scratchBuffer + scratchIndex) + OBJECT_OFFSETOF(EncodedValueDescriptor, asBits.tag));
>+            } else {
>+                m_jit.store32(recovery.gpr(), AssemblyHelpers::payloadFor((VirtualRegister)operand));
>+                m_jit.store32(AssemblyHelpers::TrustedImm32(JSValue::Int32Tag), AssemblyHelpers::tagFor((VirtualRegister)operand));
>+            }
>+            
>+            done.link(&m_jit);
>+            
>+            m_jit.move(AssemblyHelpers::TrustedImmPtr(myScratch + 1), addressGPR);
>+            m_jit.loadDouble(addressGPR, FPRInfo::fpRegT0);
>+            m_jit.loadPtr(myScratch, addressGPR);
>+                              
>+            if (exit.isVariable(index) && poisonedVirtualRegisters[exit.variableForIndex(index)])
>+                scratchIndex++;
>+            break;
>+        }
>         default:
>             break;
>         }
>@@ -378,6 +428,7 @@ void OSRExitCompiler::compileExit(const 
> 
>             case InFPR:
>             case InPair:
>+            case UInt32InGPR:
>                 m_jit.load32(reinterpret_cast<char*>(scratchBuffer + scratchIndex) + OBJECT_OFFSETOF(EncodedValueDescriptor, asBits.payload), GPRInfo::regT0);
>                 m_jit.load32(reinterpret_cast<char*>(scratchBuffer + scratchIndex) + OBJECT_OFFSETOF(EncodedValueDescriptor, asBits.tag), GPRInfo::regT1);
>                 m_jit.store32(GPRInfo::regT0, AssemblyHelpers::payloadFor((VirtualRegister)virtualRegister));
>Index: Source/JavaScriptCore/dfg/DFGOSRExitCompiler64.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGOSRExitCompiler64.cpp	(revision 102701)
>+++ Source/JavaScriptCore/dfg/DFGOSRExitCompiler64.cpp	(working copy)
>@@ -119,6 +119,7 @@ void OSRExitCompiler::compileExit(const 
>     bool haveFPRs = false;
>     bool haveConstants = false;
>     bool haveUndefined = false;
>+    bool haveUInt32s = false;
>     
>     for (int index = 0; index < exit.numberOfRecoveries(); ++index) {
>         const ValueRecovery& recovery = exit.valueRecovery(index);
>@@ -140,6 +141,7 @@ void OSRExitCompiler::compileExit(const 
>                 switch (exit.m_variables[recovery.virtualRegister()].technique()) {
>                 case InGPR:
>                 case UnboxedInt32InGPR:
>+                case UInt32InGPR:
>                 case InFPR:
>                     if (!poisonedVirtualRegisters[recovery.virtualRegister()]) {
>                         poisonedVirtualRegisters[recovery.virtualRegister()] = true;
>@@ -157,6 +159,10 @@ void OSRExitCompiler::compileExit(const 
>             haveUnboxedInt32s = true;
>             break;
>             
>+        case UInt32InGPR:
>+            haveUInt32s = true;
>+            break;
>+            
>         case InFPR:
>             haveFPRs = true;
>             break;
>@@ -180,6 +186,8 @@ void OSRExitCompiler::compileExit(const 
>         fprintf(stderr, "Displaced=%u ", numberOfDisplacedVirtualRegisters);
>     if (haveUnboxedInt32s)
>         fprintf(stderr, "UnboxedInt32 ");
>+    if (haveUInt32s)
>+        fprintf(stderr, "UInt32 ");
>     if (haveFPRs)
>         fprintf(stderr, "FPR ");
>     if (haveConstants)
>@@ -189,14 +197,14 @@ void OSRExitCompiler::compileExit(const 
>     fprintf(stderr, " ");
> #endif
>     
>-    EncodedJSValue* scratchBuffer = static_cast<EncodedJSValue*>(m_jit.globalData()->scratchBufferForSize(sizeof(EncodedJSValue) * (numberOfPoisonedVirtualRegisters + (numberOfDisplacedVirtualRegisters <= GPRInfo::numberOfRegisters ? 0 : numberOfDisplacedVirtualRegisters))));
>+    EncodedJSValue* scratchBuffer = static_cast<EncodedJSValue*>(m_jit.globalData()->scratchBufferForSize(sizeof(EncodedJSValue) * std::max(haveUInt32s ? 2u : 0u, numberOfPoisonedVirtualRegisters + (numberOfDisplacedVirtualRegisters <= GPRInfo::numberOfRegisters ? 0 : numberOfDisplacedVirtualRegisters))));
> 
>     // From here on, the code assumes that it is profitable to maximize the distance
>     // between when something is computed and when it is stored.
>     
>     // 5) Perform all reboxing of integers.
>     
>-    if (haveUnboxedInt32s) {
>+    if (haveUnboxedInt32s || haveUInt32s) {
>         for (int index = 0; index < exit.numberOfRecoveries(); ++index) {
>             const ValueRecovery& recovery = exit.valueRecovery(index);
>             switch (recovery.technique()) {
>@@ -209,6 +217,44 @@ void OSRExitCompiler::compileExit(const 
>                 m_jit.store32(AssemblyHelpers::Imm32(static_cast<uint32_t>(TagTypeNumber >> 32)), AssemblyHelpers::tagFor(static_cast<VirtualRegister>(exit.operandForIndex(index))));
>                 break;
>                 
>+            case UInt32InGPR: {
>+                // This occurs when the speculative JIT left an unsigned 32-bit integer
>+                // in a GPR. If it's positive, we can just box the int. Otherwise we
>+                // need to turn it into a boxed double.
>+                
>+                // We don't try to be clever with register allocation here; we assume
>+                // that the program is using FPRs and we don't try to figure out which
>+                // ones it is using. Instead just temporarily save fpRegT0 and then
>+                // restore it. This makes sense because this path is not cheap to begin
>+                // with, and should happen very rarely.
>+                
>+                GPRReg addressGPR = GPRInfo::regT0;
>+                if (addressGPR == recovery.gpr())
>+                    addressGPR = GPRInfo::regT1;
>+                
>+                m_jit.storePtr(addressGPR, scratchBuffer);
>+                m_jit.move(AssemblyHelpers::TrustedImmPtr(scratchBuffer + 1), addressGPR);
>+                m_jit.storeDouble(FPRInfo::fpRegT0, addressGPR);
>+                
>+                AssemblyHelpers::Jump positive = m_jit.branch32(AssemblyHelpers::GreaterThanOrEqual, recovery.gpr(), AssemblyHelpers::TrustedImm32(0));
>+
>+                m_jit.convertInt32ToDouble(recovery.gpr(), FPRInfo::fpRegT0);
>+                m_jit.addDouble(AssemblyHelpers::AbsoluteAddress(&AssemblyHelpers::twoToThe32), FPRInfo::fpRegT0);
>+                m_jit.boxDouble(FPRInfo::fpRegT0, recovery.gpr());
>+                
>+                AssemblyHelpers::Jump done = m_jit.jump();
>+                
>+                positive.link(&m_jit);
>+                
>+                m_jit.orPtr(GPRInfo::tagTypeNumberRegister, recovery.gpr());
>+                
>+                done.link(&m_jit);
>+                
>+                m_jit.loadDouble(addressGPR, FPRInfo::fpRegT0);
>+                m_jit.loadPtr(scratchBuffer, addressGPR);
>+                break;
>+            }
>+                
>             default:
>                 break;
>             }
>@@ -226,6 +272,7 @@ void OSRExitCompiler::compileExit(const 
>         switch (recovery.technique()) {
>         case InGPR:
>         case UnboxedInt32InGPR:
>+        case UInt32InGPR:
>             if (exit.isVariable(index) && poisonedVirtualRegisters[exit.variableForIndex(index)])
>                 m_jit.storePtr(recovery.gpr(), scratchBuffer + scratchIndex++);
>             else
>@@ -395,6 +442,7 @@ void OSRExitCompiler::compileExit(const 
>             switch (recovery.technique()) {
>             case InGPR:
>             case UnboxedInt32InGPR:
>+            case UInt32InGPR:
>             case InFPR:
>                 m_jit.loadPtr(scratchBuffer + scratchIndex++, GPRInfo::regT0);
>                 m_jit.storePtr(GPRInfo::regT0, AssemblyHelpers::addressFor((VirtualRegister)virtualRegister));
>Index: Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp	(revision 102701)
>+++ Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp	(working copy)
>@@ -454,7 +454,7 @@ void SpeculativeJIT::nonSpeculativeUInt3
>     JITCompiler::Jump positive = m_jit.branch32(MacroAssembler::GreaterThanOrEqual, op1.gpr(), TrustedImm32(0));
>         
>     m_jit.convertInt32ToDouble(op1.gpr(), boxer.fpr());
>-    m_jit.move(JITCompiler::TrustedImmPtr(&twoToThe32), resultPayload.gpr()); // reuse resultPayload register here.
>+    m_jit.move(JITCompiler::TrustedImmPtr(&AssemblyHelpers::twoToThe32), resultPayload.gpr()); // reuse resultPayload register here.
>     m_jit.addDouble(JITCompiler::Address(resultPayload.gpr(), 0), boxer.fpr());
>         
>     boxDouble(boxer.fpr(), resultTag.gpr(), resultPayload.gpr());
>@@ -2169,34 +2169,7 @@ void SpeculativeJIT::compile(Node& node)
>         break;
> 
>     case UInt32ToNumber: {
>-        if (!nodeCanSpeculateInteger(node.arithNodeFlags())) {
>-            // We know that this sometimes produces doubles. So produce a double every
>-            // time. This at least allows subsequent code to not have weird conditionals.
>-            
>-            IntegerOperand op1(this, node.child1());
>-            FPRTemporary result(this);
>-            
>-            GPRReg inputGPR = op1.gpr();
>-            FPRReg outputFPR = result.fpr();
>-            
>-            m_jit.convertInt32ToDouble(inputGPR, outputFPR);
>-            
>-            JITCompiler::Jump positive = m_jit.branch32(MacroAssembler::GreaterThanOrEqual, inputGPR, TrustedImm32(0));
>-            m_jit.addDouble(JITCompiler::AbsoluteAddress(&twoToThe32), outputFPR);
>-            positive.link(&m_jit);
>-            
>-            doubleResult(outputFPR, m_compileIndex);
>-            break;
>-        }
>-
>-        IntegerOperand op1(this, node.child1());
>-        GPRTemporary result(this, op1);
>-
>-        // Test the operand is positive.
>-        speculationCheck(Overflow, JSValueRegs(), NoNode, m_jit.branch32(MacroAssembler::LessThan, op1.gpr(), TrustedImm32(0)));
>-
>-        m_jit.move(op1.gpr(), result.gpr());
>-        integerResult(result.gpr(), m_compileIndex, op1.format());
>+        compileUInt32ToNumber(node);
>         break;
>     }
> 
>Index: Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp	(revision 102701)
>+++ Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp	(working copy)
>@@ -462,7 +462,7 @@ void SpeculativeJIT::nonSpeculativeUInt3
>     JITCompiler::Jump positive = m_jit.branch32(MacroAssembler::GreaterThanOrEqual, op1.gpr(), TrustedImm32(0));
>     
>     m_jit.convertInt32ToDouble(op1.gpr(), boxer.fpr());
>-    m_jit.addDouble(JITCompiler::AbsoluteAddress(&twoToThe32), boxer.fpr());
>+    m_jit.addDouble(JITCompiler::AbsoluteAddress(&AssemblyHelpers::twoToThe32), boxer.fpr());
>     
>     boxDouble(boxer.fpr(), result.gpr());
>     
>@@ -2201,34 +2201,7 @@ void SpeculativeJIT::compile(Node& node)
>         break;
> 
>     case UInt32ToNumber: {
>-        if (!nodeCanSpeculateInteger(node.arithNodeFlags())) {
>-            // We know that this sometimes produces doubles. So produce a double every
>-            // time. This at least allows subsequent code to not have weird conditionals.
>-            
>-            IntegerOperand op1(this, node.child1());
>-            FPRTemporary result(this);
>-            
>-            GPRReg inputGPR = op1.gpr();
>-            FPRReg outputFPR = result.fpr();
>-            
>-            m_jit.convertInt32ToDouble(inputGPR, outputFPR);
>-            
>-            JITCompiler::Jump positive = m_jit.branch32(MacroAssembler::GreaterThanOrEqual, inputGPR, TrustedImm32(0));
>-            m_jit.addDouble(JITCompiler::AbsoluteAddress(&twoToThe32), outputFPR);
>-            positive.link(&m_jit);
>-            
>-            doubleResult(outputFPR, m_compileIndex);
>-            break;
>-        }
>-
>-        IntegerOperand op1(this, node.child1());
>-        GPRTemporary result(this, op1);
>-
>-        // Test the operand is positive.
>-        speculationCheck(Overflow, JSValueRegs(), NoNode, m_jit.branch32(MacroAssembler::LessThan, op1.gpr(), TrustedImm32(0)));
>-
>-        m_jit.move(op1.gpr(), result.gpr());
>-        integerResult(result.gpr(), m_compileIndex, op1.format());
>+        compileUInt32ToNumber(node);
>         break;
>     }
> 
>Index: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp	(revision 102701)
>+++ Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp	(working copy)
>@@ -44,8 +44,6 @@ static double DFG_OPERATION fmodAsDFGOpe
> #define fmodAsDFGOperation fmod
> #endif
> 
>-const double SpeculativeJIT::twoToThe32 = (double)0x100000000ull;
>-
> void SpeculativeJIT::clearGenerationInfo()
> {
>     for (unsigned i = 0; i < m_generationInfo.size(); ++i)
>@@ -1459,6 +1457,56 @@ void SpeculativeJIT::compileValueToInt32
>     integerResult(result.gpr(), m_compileIndex, op1.format());
> }
> 
>+void SpeculativeJIT::compileUInt32ToNumber(Node& node)
>+{
>+    if (!nodeCanSpeculateInteger(node.arithNodeFlags())) {
>+        // We know that this sometimes produces doubles. So produce a double every
>+        // time. This at least allows subsequent code to not have weird conditionals.
>+            
>+        IntegerOperand op1(this, node.child1());
>+        FPRTemporary result(this);
>+            
>+        GPRReg inputGPR = op1.gpr();
>+        FPRReg outputFPR = result.fpr();
>+            
>+        m_jit.convertInt32ToDouble(inputGPR, outputFPR);
>+            
>+        JITCompiler::Jump positive = m_jit.branch32(MacroAssembler::GreaterThanOrEqual, inputGPR, TrustedImm32(0));
>+        m_jit.addDouble(JITCompiler::AbsoluteAddress(&AssemblyHelpers::twoToThe32), outputFPR);
>+        positive.link(&m_jit);
>+            
>+        doubleResult(outputFPR, m_compileIndex);
>+        return;
>+    }
>+
>+    IntegerOperand op1(this, node.child1());
>+    GPRTemporary result(this, op1);
>+
>+    // Test the operand is positive. This is a very special speculation check - we actually
>+    // use roll-forward speculation here, where if this fails, we jump to the baseline
>+    // instruction that follows us, rather than the one we're executing right now. We have
>+    // to do this because by this point, the original values necessary to compile whatever
>+    // operation the UInt32ToNumber originated from might be dead.
>+    speculationCheck(Overflow, JSValueRegs(), NoNode, m_jit.branch32(MacroAssembler::LessThan, op1.gpr(), TrustedImm32(0)));
>+        
>+    // Verify that we can do roll forward.
>+    ASSERT(at(m_compileIndex + 1).op == SetLocal);
>+    ASSERT(at(m_compileIndex + 1).codeOrigin == node.codeOrigin);
>+    ASSERT(at(m_compileIndex + 2).codeOrigin != node.codeOrigin);
>+        
>+    // Now do the magic.
>+    OSRExit& exit = m_jit.codeBlock()->lastOSRExit();
>+    Node& setLocal = at(m_compileIndex + 1);
>+    exit.m_codeOrigin = at(m_compileIndex + 2).codeOrigin;
>+    exit.m_lastSetOperand = setLocal.local();
>+        
>+    // Create the value recovery, and stuff it into the right place.
>+    exit.valueRecoveryForOperand(setLocal.local()) = ValueRecovery::uint32InGPR(op1.gpr());
>+
>+    m_jit.move(op1.gpr(), result.gpr());
>+    integerResult(result.gpr(), m_compileIndex, op1.format());
>+}
>+
> static void compileClampDoubleToByte(JITCompiler& jit, GPRReg result, FPRReg source, FPRReg scratch)
> {
>     // Unordered compare so we pick up NaN
>@@ -1640,7 +1688,7 @@ void SpeculativeJIT::compileGetByValOnIn
>         FPRTemporary fresult(this);
>         m_jit.convertInt32ToDouble(resultReg, fresult.fpr());
>         JITCompiler::Jump positive = m_jit.branch32(MacroAssembler::GreaterThanOrEqual, resultReg, TrustedImm32(0));
>-        m_jit.addDouble(JITCompiler::AbsoluteAddress(&twoToThe32), fresult.fpr());
>+        m_jit.addDouble(JITCompiler::AbsoluteAddress(&AssemblyHelpers::twoToThe32), fresult.fpr());
>         positive.link(&m_jit);
>         doubleResult(fresult.fpr(), m_compileIndex);
>     }
>Index: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h	(revision 102701)
>+++ Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h	(working copy)
>@@ -170,8 +170,6 @@ private:
> 
>     enum UseChildrenMode { CallUseChildren, UseChildrenCalledExplicitly };
>     
>-    static const double twoToThe32;
>-
> public:
>     SpeculativeJIT(JITCompiler&);
> 
>@@ -1969,6 +1967,7 @@ private:
>     void compileGetCharCodeAt(Node&);
>     void compileGetByValOnString(Node&);
>     void compileValueToInt32(Node&);
>+    void compileUInt32ToNumber(Node&);
>     void compileGetByValOnByteArray(Node&);
>     void compilePutByValForByteArray(GPRReg base, GPRReg property, Node&);
>     void compileArithMul(Node&);
>Index: LayoutTests/ChangeLog
>===================================================================
>--- LayoutTests/ChangeLog	(revision 102720)
>+++ LayoutTests/ChangeLog	(working copy)
>@@ -1,3 +1,15 @@
>+2011-12-13  Filip Pizlo  <fpizlo@apple.com>
>+
>+        DFG OSR exit for UInt32ToNumber should roll forward, not roll backward
>+        https://bugs.webkit.org/show_bug.cgi?id=74463
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * fast/js/dfg-uint32-to-number-expected.txt: Added.
>+        * fast/js/dfg-uint32-to-number.html: Added.
>+        * fast/js/script-tests/dfg-uint32-to-number.js: Added.
>+        (foo):
>+
> 2011-12-13  Tony Chang  <tony@chromium.org>
> 
>         Rolling out r102461, a change to a layout test to try to remove flakiness on Chromium.
>Index: LayoutTests/fast/js/dfg-uint32-to-number-expected.txt
>===================================================================
>--- LayoutTests/fast/js/dfg-uint32-to-number-expected.txt	(revision 0)
>+++ LayoutTests/fast/js/dfg-uint32-to-number-expected.txt	(revision 0)
>@@ -0,0 +1,13 @@
>+This tests that if the DFG fails to convert a uint32 to a number, it will OSR exit correctly.
>+
>+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>+
>+
>+PASS result is 124500
>+PASS foo({f:2147483648}, {f:32}) is 2147483648
>+PASS foo({f:2147483648}, {f:31}) is 1
>+PASS foo({f:2147483648}, {f:30}) is 2
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+
>Index: LayoutTests/fast/js/dfg-uint32-to-number.html
>===================================================================
>--- LayoutTests/fast/js/dfg-uint32-to-number.html	(revision 0)
>+++ LayoutTests/fast/js/dfg-uint32-to-number.html	(revision 0)
>@@ -0,0 +1,10 @@
>+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
>+<html>
>+<head>
>+<script src="resources/js-test-pre.js"></script>
>+</head>
>+<body>
>+<script src="script-tests/dfg-uint32-to-number.js"></script>
>+<script src="resources/js-test-post.js"></script>
>+</body>
>+</html>
>Index: LayoutTests/fast/js/script-tests/dfg-uint32-to-number.js
>===================================================================
>--- LayoutTests/fast/js/script-tests/dfg-uint32-to-number.js	(revision 0)
>+++ LayoutTests/fast/js/script-tests/dfg-uint32-to-number.js	(revision 0)
>@@ -0,0 +1,19 @@
>+description(
>+"This tests that if the DFG fails to convert a uint32 to a number, it will OSR exit correctly."
>+);
>+
>+function foo(a,b) {
>+    return a.f >>> b.f;
>+}
>+
>+var result = 0;
>+for (var i = 0; i < 1000; ++i) {
>+    result += foo({f:i + 0.5}, {f:2});
>+}
>+
>+shouldBe("result", "124500");
>+
>+shouldBe("foo({f:2147483648}, {f:32})", "2147483648");
>+shouldBe("foo({f:2147483648}, {f:31})", "1");
>+shouldBe("foo({f:2147483648}, {f:30})", "2");
>+

Flags:

barraclough: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 74463: 119109