Currently, when we compile an OSR exit, we just find a single Stackmap::Record that matches the stackmapID. But there may be many such records. We need the actual one that corresponds to the proper OSR exit.
Created attachment 262967 [details] patch
Attachment 262967 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/ftl/FTLOSRExit.cpp:48: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4] ERROR: Source/JavaScriptCore/ftl/FTLOSRExit.cpp:49: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4] ERROR: Source/JavaScriptCore/ftl/FTLOSRExit.cpp:50: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4] ERROR: Source/JavaScriptCore/ftl/FTLOSRExit.cpp:54: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4] ERROR: Source/JavaScriptCore/ftl/FTLCompile.cpp:393: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] Total errors found: 5 in 10 files If any of these errors are false positives, please file a bug against check-webkit-style.
Oops ... I closed the wrong bug. Re-opening.
Comment on attachment 262967 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=262967&action=review > Source/JavaScriptCore/ChangeLog:12 > + a signle OSR exit data structure. Then, when we compiled an OSR exit, we "single" => "single".
Comment on attachment 262967 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=262967&action=review > Source/JavaScriptCore/ftl/FTLJITCode.h:89 > + SegmentedVector<OSRExitDescriptor, 8> osrExitDescriptors; This should probably go to FTL::State since it's not used after FTL::compile(). > Source/JavaScriptCore/ftl/FTLStackMaps.h:129 > - typedef HashMap<uint32_t, Vector<Record>, WTF::IntHash<uint32_t>, WTF::UnsignedWithZeroKeyHashTraits<uint32_t>> RecordMap; > + struct RecordMapValue { > + Vector<Record> records; > + Vector<uint32_t> indices; > + }; > + typedef HashMap<uint32_t, RecordMapValue, WTF::IntHash<uint32_t>, WTF::UnsignedWithZeroKeyHashTraits<uint32_t>> RecordMap; What about using Vector<RecordAndIndex> instead of RecordMapValue? It's usually better to have one vector that contains a tuple, than a tuple containing multiple vectors.
landed in: http://trac.webkit.org/changeset/191313
(In reply to comment #5) > Comment on attachment 262967 [details] > patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=262967&action=review > > > Source/JavaScriptCore/ftl/FTLJITCode.h:89 > > + SegmentedVector<OSRExitDescriptor, 8> osrExitDescriptors; > > This should probably go to FTL::State since it's not used after > FTL::compile(). Fil and I spoke over IRC: this is actually used after FTL::compile() because each FTL::OSRExit has a reference to an item in this vector. So I'm keeping it in FTL::JITCode. > > > Source/JavaScriptCore/ftl/FTLStackMaps.h:129 > > - typedef HashMap<uint32_t, Vector<Record>, WTF::IntHash<uint32_t>, WTF::UnsignedWithZeroKeyHashTraits<uint32_t>> RecordMap; > > + struct RecordMapValue { > > + Vector<Record> records; > > + Vector<uint32_t> indices; > > + }; > > + typedef HashMap<uint32_t, RecordMapValue, WTF::IntHash<uint32_t>, WTF::UnsignedWithZeroKeyHashTraits<uint32_t>> RecordMap; > > What about using Vector<RecordAndIndex> instead of RecordMapValue? It's > usually better to have one vector that contains a tuple, than a tuple > containing multiple vectors.