I think some of the exception handling state and code in FTLCompile can use a nice abstraction or just a different file to live in.
This should also handle the case where it shouldn't bother generating call site indices for tail calls.
Created attachment 265212 [details] patch
Attachment 265212 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/ftl/FTLExceptionHandlerManager.h:45: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] ERROR: Source/JavaScriptCore/ftl/FTLExceptionHandlerManager.h:54: The parameter name "state" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 2 in 8 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 265212 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=265212&action=review r=me with comments. > Source/JavaScriptCore/CMakeLists.txt:-993 > + ftl/FTLDataSection.cpp > ftl/FTLDWARFDebugLineInfo.cpp > ftl/FTLDWARFRegister.cpp > - ftl/FTLDataSection.cpp nit: Technically, FTLDWARFRegister.cpp should come before FTLDataSection.cpp according to ASCII sort order. However, I see that the vcxproj files do it differently. I'll let you decide on which is best. > Source/JavaScriptCore/ftl/FTLExceptionHandlerManager.cpp:122 > + if (!exit.m_exceptionHandlerCallSiteIndex) > + exit.m_exceptionHandlerCallSiteIndex = m_state.jitCode->common.addUniqueCallSiteIndex(origin); This part is new, and not present in the original generateOrGetAlreadyGeneratedCallSiteIndex(). I see that this used to be assigned at line 613 in fixFunctionBasedOnStackMaps(), and it still is assigned there. Is there a reason that we need to have a separate assignment here? Can it be one or the other (and not both)?
Comment on attachment 265212 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=265212&action=review >> Source/JavaScriptCore/ftl/FTLExceptionHandlerManager.cpp:122 >> + exit.m_exceptionHandlerCallSiteIndex = m_state.jitCode->common.addUniqueCallSiteIndex(origin); > > This part is new, and not present in the original generateOrGetAlreadyGeneratedCallSiteIndex(). I see that this used to be assigned at line 613 in fixFunctionBasedOnStackMaps(), and it still is assigned there. Is there a reason that we need to have a separate assignment here? Can it be one or the other (and not both)? You're right. We should never need to generate one here. I'll just assert that we have a valid callsiteindex here.
landed in: http://trac.webkit.org/changeset/192273