speculateNeitherDoubleNorStringNorHeapBigInt should only have a single JSType branch
Created attachment 433934 [details] Patch
Comment on attachment 433934 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=433934&action=review > Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:11792 > + DFG_TYPE_CHECK(regs, edge, ~(SpecString | SpecHeapBigInt), m_jit.branchIfType(regs.payloadGPR(), JSTypeRange { StringType, HeapBigIntType })); It seems like this should be SpecAnyString but maybe I was missing something?
Comment on attachment 433934 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=433934&action=review >> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:11792 >> + DFG_TYPE_CHECK(regs, edge, ~(SpecString | SpecHeapBigInt), m_jit.branchIfType(regs.payloadGPR(), JSTypeRange { StringType, HeapBigIntType })); > > It seems like this should be SpecAnyString but maybe I was missing something? Nvm, I confused myself, I was thinking that SpecString independent from SpecStringIdent and SpecStringVar...
Comment on attachment 433934 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=433934&action=review r=me > Source/JavaScriptCore/ChangeLog:7 > + Maybe add some small comment here explaining your trick for people scanning through the Changelog. >> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:11792 >> + DFG_TYPE_CHECK(regs, edge, ~(SpecString | SpecHeapBigInt), m_jit.branchIfType(regs.payloadGPR(), JSTypeRange { StringType, HeapBigIntType })); > > It seems like this should be SpecAnyString but maybe I was missing something? SpecAnyString does not exist, SpecString = SpecStringIdent | SpecStringVar is the union type. > Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:-19012 > - return m_out.equal( I think I would have kept this version of isType intact, and called it from the JSTypeRange version when range.last == range.first, but I'm ok with your approach even if I find it less intuitive.
Comment on attachment 433934 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=433934&action=review >> Source/JavaScriptCore/ChangeLog:7 >> + > > Maybe add some small comment here explaining your trick for people scanning through the Changelog. Sure. >> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:-19012 >> - return m_out.equal( > > I think I would have kept this version of isType intact, and called it from the JSTypeRange version when range.last == range.first, but I'm ok with your approach even if I find it less intuitive. I just followed how the AssemblyHelpers version did it for consistency. I don't have strong feelings either way though.
Created attachment 433942 [details] Patch for landing
Committed r280149 (239846@main): <https://commits.webkit.org/239846@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 433942 [details].
<rdar://problem/80906151>