I think this is the right way to go, but I'm still thinking it over.
Created attachment 216717 [details] it begins ugh this will be painful
Created attachment 216862 [details] slowly getting there
Created attachment 216966 [details] I wrote the code For the most part, anyway.
Created attachment 216971 [details] compiles with FTL disabled
Created attachment 216974 [details] it builds
Created attachment 216986 [details] starting to pass some tests
Created attachment 216998 [details] making some progress
Created attachment 217073 [details] the patch
Attachment 217073 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'Source/JavaScriptCore/ChangeLog', u'Source/JavaScriptCore/GNUmakefile.list.am', u'Source/JavaScriptCore/JavaScriptCore.vcxproj/JavaScriptCore.vcxproj', u'Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj', u'Source/JavaScriptCore/bytecode/BytecodeBasicBlock.h', u'Source/JavaScriptCore/bytecode/BytecodeLivenessAnalysis.cpp', u'Source/JavaScriptCore/bytecode/BytecodeLivenessAnalysis.h', u'Source/JavaScriptCore/bytecode/BytecodeLivenessAnalysisInlines.h', u'Source/JavaScriptCore/bytecode/CodeBlock.h', u'Source/JavaScriptCore/bytecode/CodeOrigin.cpp', u'Source/JavaScriptCore/bytecode/FullBytecodeLiveness.h', u'Source/JavaScriptCore/dfg/DFGAvailability.cpp', u'Source/JavaScriptCore/dfg/DFGAvailability.h', u'Source/JavaScriptCore/dfg/DFGBasicBlock.h', u'Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp', u'Source/JavaScriptCore/dfg/DFGDisassembler.cpp', u'Source/JavaScriptCore/dfg/DFGFlushFormat.cpp', u'Source/JavaScriptCore/dfg/DFGFlushFormat.h', u'Source/JavaScriptCore/dfg/DFGFlushedAt.cpp', u'Source/JavaScriptCore/dfg/DFGFlushedAt.h', u'Source/JavaScriptCore/dfg/DFGGraph.cpp', u'Source/JavaScriptCore/dfg/DFGGraph.h', u'Source/JavaScriptCore/dfg/DFGMovHintCountingPhase.cpp', u'Source/JavaScriptCore/dfg/DFGMovHintCountingPhase.h', u'Source/JavaScriptCore/dfg/DFGOSRAvailabilityAnalysisPhase.cpp', u'Source/JavaScriptCore/dfg/DFGOSRAvailabilityAnalysisPhase.h', u'Source/JavaScriptCore/dfg/DFGPlan.cpp', u'Source/JavaScriptCore/dfg/DFGResurrectionForValidationPhase.cpp', u'Source/JavaScriptCore/dfg/DFGResurrectionForValidationPhase.h', u'Source/JavaScriptCore/dfg/DFGSSAConversionPhase.cpp', u'Source/JavaScriptCore/dfg/DFGValueSource.h', u'Source/JavaScriptCore/dfg/DFGVariableAccessData.h', u'Source/JavaScriptCore/dfg/DFGVariableAccessDataDump.cpp', u'Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp', u'Source/JavaScriptCore/ftl/FTLOSRExit.cpp', u'Source/JavaScriptCore/ftl/FTLOutput.h', u'Source/JavaScriptCore/ftl/FTLValueSource.cpp', u'Source/JavaScriptCore/ftl/FTLValueSource.h', u'Source/JavaScriptCore/runtime/DumpContext.cpp', u'Source/JavaScriptCore/runtime/DumpContext.h', u'Source/JavaScriptCore/runtime/Options.h', u'Source/JavaScriptCore/runtime/SymbolTable.h', u'Tools/ChangeLog', u'Tools/Scripts/run-jsc-stress-tests']" exit_code: 1 Source/JavaScriptCore/dfg/DFGAvailability.h:64: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] Source/JavaScriptCore/dfg/DFGGraph.h:787: The parameter name "codeBlock" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/dfg/DFGGraph.h:788: The parameter name "inlineCallFrame" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/dfg/DFGGraph.h:789: The parameter name "codeOrigin" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/bytecode/BytecodeLivenessAnalysis.h:59: The parameter name "codeBlock" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/bytecode/BytecodeLivenessAnalysis.h:60: The parameter name "codeBlock" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/bytecode/BytecodeLivenessAnalysis.h:61: The parameter name "codeBlock" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 7 in 44 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 217073 [details] the patch Attachment 217073 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/24328008
Comment on attachment 217073 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=217073&action=review r=me > Source/JavaScriptCore/dfg/DFGGraph.cpp:676 > + if (reg.offset() >= JSStack::CallFrameHeaderSize) > + return true; Maybe we should ASSERT this instead, since it should be impossible to take this branch. > Source/JavaScriptCore/dfg/DFGMovHintCountingPhase.h:45 > +// Computes VariableAccessData::m_movHintCount. Variables that are assigned only once can get > +// treated specially. For example, OSR exit in the FTL can allow the format used for such > +// nodes BasicBlock::ssa->availabiltiyAtHead/Tail. This is a forward flow type inference > +// over MovHints. At the time when MovHints are introduced, we were certain that at any > +// point in the program where a bytecode variable had been live, all of the MovHints that > +// flowed into that point for that variable had the same FlushFormat. This analysis will tell > +// you what that format would have been. Comment seems whacky. > Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp:3960 > + else if (direction == ForwardSpeculation) { else? > Source/JavaScriptCore/ftl/FTLOSRExit.cpp:76 > - > + Revert!
Comment on attachment 217073 [details] the patch Attachment 217073 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/24508008
Comment on attachment 217073 [details] the patch This is a performance regression. It's because of the bytecode shadow locals. After talking with mhahnenberg and atrick, I think I'm going to use a different approach.
Created attachment 217111 [details] work in progress - new approach
Created attachment 217113 [details] the patch
Attachment 217113 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'Source/JavaScriptCore/ChangeLog', u'Source/JavaScriptCore/GNUmakefile.list.am', u'Source/JavaScriptCore/JavaScriptCore.vcxproj/JavaScriptCore.vcxproj', u'Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj', u'Source/JavaScriptCore/bytecode/BytecodeBasicBlock.h', u'Source/JavaScriptCore/bytecode/BytecodeLivenessAnalysis.cpp', u'Source/JavaScriptCore/bytecode/BytecodeLivenessAnalysis.h', u'Source/JavaScriptCore/bytecode/BytecodeLivenessAnalysisInlines.h', u'Source/JavaScriptCore/bytecode/CodeBlock.h', u'Source/JavaScriptCore/bytecode/CodeOrigin.cpp', u'Source/JavaScriptCore/bytecode/FullBytecodeLiveness.h', u'Source/JavaScriptCore/dfg/DFGAvailability.cpp', u'Source/JavaScriptCore/dfg/DFGAvailability.h', u'Source/JavaScriptCore/dfg/DFGBasicBlock.h', u'Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp', u'Source/JavaScriptCore/dfg/DFGDisassembler.cpp', u'Source/JavaScriptCore/dfg/DFGFlushFormat.cpp', u'Source/JavaScriptCore/dfg/DFGFlushFormat.h', u'Source/JavaScriptCore/dfg/DFGFlushedAt.cpp', u'Source/JavaScriptCore/dfg/DFGFlushedAt.h', u'Source/JavaScriptCore/dfg/DFGGraph.cpp', u'Source/JavaScriptCore/dfg/DFGGraph.h', u'Source/JavaScriptCore/dfg/DFGOSRAvailabilityAnalysisPhase.cpp', u'Source/JavaScriptCore/dfg/DFGOSRAvailabilityAnalysisPhase.h', u'Source/JavaScriptCore/dfg/DFGPlan.cpp', u'Source/JavaScriptCore/dfg/DFGResurrectionForValidationPhase.cpp', u'Source/JavaScriptCore/dfg/DFGResurrectionForValidationPhase.h', u'Source/JavaScriptCore/dfg/DFGSSAConversionPhase.cpp', u'Source/JavaScriptCore/dfg/DFGValueSource.h', u'Source/JavaScriptCore/dfg/DFGVariableAccessData.h', u'Source/JavaScriptCore/ftl/FTLExitValue.cpp', u'Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp', u'Source/JavaScriptCore/ftl/FTLOSRExit.cpp', u'Source/JavaScriptCore/ftl/FTLOutput.h', u'Source/JavaScriptCore/ftl/FTLValueSource.cpp', u'Source/JavaScriptCore/ftl/FTLValueSource.h', u'Source/JavaScriptCore/llvm/LLVMAPIFunctions.h', u'Source/JavaScriptCore/runtime/DumpContext.cpp', u'Source/JavaScriptCore/runtime/DumpContext.h', u'Source/JavaScriptCore/runtime/Options.h', u'Source/JavaScriptCore/runtime/SymbolTable.h']" exit_code: 1 Source/JavaScriptCore/dfg/DFGAvailability.h:84: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] Source/JavaScriptCore/dfg/DFGPlan.cpp:269: Should have a space between // and comment [whitespace/comments] [4] Source/JavaScriptCore/dfg/DFGGraph.h:787: The parameter name "codeBlock" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/dfg/DFGGraph.h:788: The parameter name "inlineCallFrame" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/dfg/DFGGraph.h:789: The parameter name "codeOrigin" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/bytecode/BytecodeLivenessAnalysis.h:59: The parameter name "codeBlock" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/bytecode/BytecodeLivenessAnalysis.h:60: The parameter name "codeBlock" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/bytecode/BytecodeLivenessAnalysis.h:61: The parameter name "codeBlock" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 8 in 38 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 217114 [details] the patch
Attachment 217114 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'Source/JavaScriptCore/ChangeLog', u'Source/JavaScriptCore/GNUmakefile.list.am', u'Source/JavaScriptCore/JavaScriptCore.vcxproj/JavaScriptCore.vcxproj', u'Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj', u'Source/JavaScriptCore/bytecode/BytecodeBasicBlock.h', u'Source/JavaScriptCore/bytecode/BytecodeLivenessAnalysis.cpp', u'Source/JavaScriptCore/bytecode/BytecodeLivenessAnalysis.h', u'Source/JavaScriptCore/bytecode/BytecodeLivenessAnalysisInlines.h', u'Source/JavaScriptCore/bytecode/CodeBlock.h', u'Source/JavaScriptCore/bytecode/CodeOrigin.cpp', u'Source/JavaScriptCore/bytecode/FullBytecodeLiveness.h', u'Source/JavaScriptCore/dfg/DFGAvailability.cpp', u'Source/JavaScriptCore/dfg/DFGAvailability.h', u'Source/JavaScriptCore/dfg/DFGBasicBlock.h', u'Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp', u'Source/JavaScriptCore/dfg/DFGDisassembler.cpp', u'Source/JavaScriptCore/dfg/DFGFlushFormat.cpp', u'Source/JavaScriptCore/dfg/DFGFlushFormat.h', u'Source/JavaScriptCore/dfg/DFGFlushedAt.cpp', u'Source/JavaScriptCore/dfg/DFGFlushedAt.h', u'Source/JavaScriptCore/dfg/DFGGraph.cpp', u'Source/JavaScriptCore/dfg/DFGGraph.h', u'Source/JavaScriptCore/dfg/DFGOSRAvailabilityAnalysisPhase.cpp', u'Source/JavaScriptCore/dfg/DFGOSRAvailabilityAnalysisPhase.h', u'Source/JavaScriptCore/dfg/DFGPlan.cpp', u'Source/JavaScriptCore/dfg/DFGResurrectionForValidationPhase.cpp', u'Source/JavaScriptCore/dfg/DFGResurrectionForValidationPhase.h', u'Source/JavaScriptCore/dfg/DFGSSAConversionPhase.cpp', u'Source/JavaScriptCore/dfg/DFGValueSource.h', u'Source/JavaScriptCore/dfg/DFGVariableAccessData.h', u'Source/JavaScriptCore/ftl/FTLExitValue.cpp', u'Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp', u'Source/JavaScriptCore/ftl/FTLOSRExit.cpp', u'Source/JavaScriptCore/ftl/FTLOutput.h', u'Source/JavaScriptCore/ftl/FTLValueSource.cpp', u'Source/JavaScriptCore/ftl/FTLValueSource.h', u'Source/JavaScriptCore/llvm/LLVMAPIFunctions.h', u'Source/JavaScriptCore/runtime/DumpContext.cpp', u'Source/JavaScriptCore/runtime/DumpContext.h', u'Source/JavaScriptCore/runtime/Options.h', u'Source/JavaScriptCore/runtime/SymbolTable.h']" exit_code: 1 Source/JavaScriptCore/dfg/DFGAvailability.h:84: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] Source/JavaScriptCore/dfg/DFGPlan.cpp:269: Should have a space between // and comment [whitespace/comments] [4] Source/JavaScriptCore/dfg/DFGGraph.h:787: The parameter name "codeBlock" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/dfg/DFGGraph.h:788: The parameter name "inlineCallFrame" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/dfg/DFGGraph.h:789: The parameter name "codeOrigin" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/bytecode/BytecodeLivenessAnalysis.h:59: The parameter name "codeBlock" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/bytecode/BytecodeLivenessAnalysis.h:60: The parameter name "codeBlock" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/bytecode/BytecodeLivenessAnalysis.h:61: The parameter name "codeBlock" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 8 in 38 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 217114 [details] the patch Attachment 217114 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/21948319
Comment on attachment 217114 [details] the patch Attachment 217114 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/21948321
Comment on attachment 217114 [details] the patch Attachment 217114 [details] did not pass gtk-ews (gtk): Output: http://webkit-queues.appspot.com/results/23678257
Created attachment 217116 [details] the patch
Attachment 217116 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'Source/JavaScriptCore/ChangeLog', u'Source/JavaScriptCore/GNUmakefile.list.am', u'Source/JavaScriptCore/JavaScriptCore.vcxproj/JavaScriptCore.vcxproj', u'Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj', u'Source/JavaScriptCore/bytecode/BytecodeBasicBlock.h', u'Source/JavaScriptCore/bytecode/BytecodeLivenessAnalysis.cpp', u'Source/JavaScriptCore/bytecode/BytecodeLivenessAnalysis.h', u'Source/JavaScriptCore/bytecode/BytecodeLivenessAnalysisInlines.h', u'Source/JavaScriptCore/bytecode/CodeBlock.h', u'Source/JavaScriptCore/bytecode/CodeOrigin.cpp', u'Source/JavaScriptCore/bytecode/FullBytecodeLiveness.h', u'Source/JavaScriptCore/dfg/DFGAvailability.cpp', u'Source/JavaScriptCore/dfg/DFGAvailability.h', u'Source/JavaScriptCore/dfg/DFGBasicBlock.h', u'Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp', u'Source/JavaScriptCore/dfg/DFGDisassembler.cpp', u'Source/JavaScriptCore/dfg/DFGFlushFormat.cpp', u'Source/JavaScriptCore/dfg/DFGFlushFormat.h', u'Source/JavaScriptCore/dfg/DFGFlushedAt.cpp', u'Source/JavaScriptCore/dfg/DFGFlushedAt.h', u'Source/JavaScriptCore/dfg/DFGGraph.cpp', u'Source/JavaScriptCore/dfg/DFGGraph.h', u'Source/JavaScriptCore/dfg/DFGOSRAvailabilityAnalysisPhase.cpp', u'Source/JavaScriptCore/dfg/DFGOSRAvailabilityAnalysisPhase.h', u'Source/JavaScriptCore/dfg/DFGPlan.cpp', u'Source/JavaScriptCore/dfg/DFGResurrectionForValidationPhase.cpp', u'Source/JavaScriptCore/dfg/DFGResurrectionForValidationPhase.h', u'Source/JavaScriptCore/dfg/DFGSSAConversionPhase.cpp', u'Source/JavaScriptCore/dfg/DFGValueSource.h', u'Source/JavaScriptCore/dfg/DFGVariableAccessData.h', u'Source/JavaScriptCore/ftl/FTLExitValue.cpp', u'Source/JavaScriptCore/ftl/FTLInlineCacheSize.cpp', u'Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp', u'Source/JavaScriptCore/ftl/FTLOutput.h', u'Source/JavaScriptCore/ftl/FTLValueSource.cpp', u'Source/JavaScriptCore/ftl/FTLValueSource.h', u'Source/JavaScriptCore/llvm/LLVMAPIFunctions.h', u'Source/JavaScriptCore/runtime/DumpContext.cpp', u'Source/JavaScriptCore/runtime/DumpContext.h', u'Source/JavaScriptCore/runtime/Options.h', u'Source/JavaScriptCore/runtime/SymbolTable.h']" exit_code: 1 Source/JavaScriptCore/dfg/DFGAvailability.h:84: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] Source/JavaScriptCore/dfg/DFGGraph.h:786: The parameter name "codeBlock" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/dfg/DFGGraph.h:787: The parameter name "inlineCallFrame" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/dfg/DFGGraph.h:788: The parameter name "codeOrigin" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/bytecode/BytecodeLivenessAnalysis.h:59: The parameter name "codeBlock" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/bytecode/BytecodeLivenessAnalysis.h:60: The parameter name "codeBlock" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/bytecode/BytecodeLivenessAnalysis.h:61: The parameter name "codeBlock" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 7 in 38 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 217120 [details] the patch fix style
Created attachment 217121 [details] the patch
(In reply to comment #11) > (From update of attachment 217073 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=217073&action=review > > r=me > > > Source/JavaScriptCore/dfg/DFGGraph.cpp:676 > > + if (reg.offset() >= JSStack::CallFrameHeaderSize) > > + return true; > > Maybe we should ASSERT this instead, since it should be impossible to take this branch. Yup. > > > Source/JavaScriptCore/dfg/DFGMovHintCountingPhase.h:45 > > +// Computes VariableAccessData::m_movHintCount. Variables that are assigned only once can get > > +// treated specially. For example, OSR exit in the FTL can allow the format used for such > > +// nodes BasicBlock::ssa->availabiltiyAtHead/Tail. This is a forward flow type inference > > +// over MovHints. At the time when MovHints are introduced, we were certain that at any > > +// point in the program where a bytecode variable had been live, all of the MovHints that > > +// flowed into that point for that variable had the same FlushFormat. This analysis will tell > > +// you what that format would have been. > > Comment seems whacky. I killed this phase. ;-) > > > Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp:3960 > > + else if (direction == ForwardSpeculation) { > > else? Yeah. > > > Source/JavaScriptCore/ftl/FTLOSRExit.cpp:76 > > - > > + > > Revert! Yup!
Comment on attachment 217121 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=217121&action=review > Source/JavaScriptCore/bytecode/FullBytecodeLiveness.h:35 > +typedef HashMap<unsigned, FastBitVector, WTF::IntHash<unsigned>, WTF::UnsignedWithZeroKeyHashTraits<unsigned> > BytecodeToBitmapMap; No need for the space between the trailing >>.
Landed in http://trac.webkit.org/changeset/159394