Changeset 161399 in webkit for trunk/Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp

Timestamp:

Jan 6, 2014, 8:52:48 PM (12 years ago)

Author:

[email protected]

Message:

Make the different flavors of integer arithmetic more explicit, and don't rely on (possibly stale) results of the backwards propagator to decide integer arithmetic semantics
​https://bugs.webkit.org/show_bug.cgi?id=125519

Reviewed by Geoffrey Garen.

Adds the Arith::Mode enum to arithmetic nodes, which makes it explicit what sorts of
checks and overflows the node should do. Previously this would be deduced from
backwards analysis results.

This also makes "unchecked" variants really mean that you want the int32 wrapped
result, so ArithIMul is now done in terms of ArithMul(Unchecked). That means that the
constant folder needs to compute exactly the result implied by ArithMode, instead of
just folding the double result.

• CMakeLists.txt:
• GNUmakefile.list.am:
• JavaScriptCore.vcxproj/JavaScriptCore.vcxproj:
• JavaScriptCore.xcodeproj/project.pbxproj:
• dfg/DFGAbstractInterpreterInlines.h:

(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):

• dfg/DFGArithMode.cpp: Added.

(WTF::printInternal):

• dfg/DFGArithMode.h: Added.

(JSC::DFG::doesOverflow):
(JSC::DFG::shouldCheckOverflow):
(JSC::DFG::shouldCheckNegativeZero):

• dfg/DFGCSEPhase.cpp:

(JSC::DFG::CSEPhase::pureCSE):
(JSC::DFG::CSEPhase::performNodeCSE):

• dfg/DFGConstantFoldingPhase.cpp:

(JSC::DFG::ConstantFoldingPhase::foldConstants):

• dfg/DFGFixupPhase.cpp:

(JSC::DFG::FixupPhase::fixupNode):
(JSC::DFG::FixupPhase::attemptToMakeIntegerAdd):

• dfg/DFGGraph.cpp:

(JSC::DFG::Graph::dump):

• dfg/DFGNode.h:

(JSC::DFG::Node::Node):
(JSC::DFG::Node::hasArithMode):
(JSC::DFG::Node::arithMode):
(JSC::DFG::Node::setArithMode):

• dfg/DFGSpeculativeJIT.cpp:

(JSC::DFG::SpeculativeJIT::compileUInt32ToNumber):
(JSC::DFG::SpeculativeJIT::compileDoubleAsInt32):
(JSC::DFG::SpeculativeJIT::compileAdd):
(JSC::DFG::SpeculativeJIT::compileArithSub):
(JSC::DFG::SpeculativeJIT::compileArithNegate):
(JSC::DFG::SpeculativeJIT::compileArithMul):
(JSC::DFG::SpeculativeJIT::compileArithDiv):
(JSC::DFG::SpeculativeJIT::compileArithMod):

• dfg/DFGSpeculativeJIT.h:
• dfg/DFGSpeculativeJIT32_64.cpp:

(JSC::DFG::SpeculativeJIT::compile):

• dfg/DFGSpeculativeJIT64.cpp:

(JSC::DFG::SpeculativeJIT::compile):

• ftl/FTLLowerDFGToLLVM.cpp:

(JSC::FTL::LowerDFGToLLVM::compileAddSub):
(JSC::FTL::LowerDFGToLLVM::compileArithMul):
(JSC::FTL::LowerDFGToLLVM::compileArithDivMod):
(JSC::FTL::LowerDFGToLLVM::compileArithNegate):
(JSC::FTL::LowerDFGToLLVM::compileUInt32ToNumber):

File:

• trunk/Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp (modified) (1 diff)

trunk/Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp

@@ -356 +356 @@
                 
             m_interpreter.execute(indexInBlock);
+            if (!m_state.isValid()) {
+                // If we invalidated then we shouldn't attempt to constant-fold. Here's an
+                // example:
+                //
+                //     c: JSConstant(4.2)
+                //     x: ValueToInt32(Check:Int32:@const)
+                //
+                // It would be correct for an analysis to assume that execution cannot
+                // proceed past @x. Therefore, constant-folding @x could be rather bad. But,
+                // the CFA may report that it found a constant even though it also reported
+                // that everything has been invalidated. This will only happen in a couple of
+                // the constant folding cases; most of them are also separately defensive
+                // about such things.
+                break;
+            }
             if (!node->shouldGenerate() || m_state.didClobber() || node->hasConstant())
                 continue;