...
Created attachment 397608 [details] Patch
Comment on attachment 397608 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=397608&action=review > Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:1112 > + if (arithProfile->didObserveBigInt32()) > + node->mergeFlags(NodeMayHaveBigInt32Result); > + if (arithProfile->didObserveHeapBigInt() || m_inlineStackTop->m_exitProfile.hasExitSite(m_currentIndex, BigInt32Overflow)) > + node->mergeFlags(NodeMayHaveHeapBigIntResult); We are currently telling this but not using it yet for ValueDiv.
I'll do Inc / Dec too.
Created attachment 397609 [details] Patch
Created attachment 397610 [details] Patch
Created attachment 397611 [details] Patch
Created attachment 397612 [details] Patch
Committed r260730: <https://trac.webkit.org/changeset/260730>
<rdar://problem/62408278>
Comment on attachment 397612 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=397612&action=review > Source/JavaScriptCore/dfg/DFGFixupPhase.cpp:328 > + // Shift can throw RangeError. is this specified though? But maybe it's best to be consistent amongst the tiers. > Source/JavaScriptCore/dfg/DFGFixupPhase.cpp:763 > + if (Node::shouldSpeculateHeapBigInt(node->child1().node(), node->child2().node())) { nice: I commented on this in Robin's patch but I'm glad you're fixing it here > Source/JavaScriptCore/dfg/DFGNode.h:2880 > + bool canSpeculateBigInt32(RareCaseProfilingSource source) we're not using the rare case profiling for this, so do we really need it passed around? > Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4136 > + // FIXME: Introduce another BigInt32 code generation: binary use kinds are BigIntUse32, but result is SpecAnyInt and accepting overflow. Not super important, but it's not SpecAnyInt, it's: SpecAnyInt => SpecAnyBigInt or SpecBigInt > Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4986 > + // FIXME: Introduce another BigInt32 code generation: binary use kinds are BigIntUse32, but result is SpecAnyInt and accepting overflow. ditto > Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:2146 > + // FIXME: Introduce another BigInt32 code generation: binary use kinds are BigIntUse32, but result is SpecAnyInt and accepting overflow. ditto
Comment on attachment 397612 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=397612&action=review >> Source/JavaScriptCore/dfg/DFGFixupPhase.cpp:328 >> + // Shift can throw RangeError. > > is this specified though? But maybe it's best to be consistent amongst the tiers. I think, to make it consistent with JSString etc. we should throw OOM error instead of RangeError. And in that case, we can clear MustGenerate. https://bugs.webkit.org/show_bug.cgi?id=211111 >> Source/JavaScriptCore/dfg/DFGNode.h:2880 >> + bool canSpeculateBigInt32(RareCaseProfilingSource source) > > we're not using the rare case profiling for this, so do we really need it passed around? Currently not using it. But I left it for the future extension. 168 // We always attempt to produce BigInt32 as much as possible. If we see HeapBigInt, we observed overflow. 169 // FIXME: Extend this information by tiered profiling (from Baseline, DFG etc.). 170 // https://bugs.webkit.org/show_bug.cgi?id=211039