Nodes that perform speculations that are necessary to prove that the equivalent bytecode operation is pure must be marked NodeMustGenerate. MultiGetByOffset checks the structure of its input. If the incoming object didn't have that structure, then the equivalent bytecode operation (get_by_id) might do something effectful. But, it's not marked NodeMustGenerate. We should fix that.
Created attachment 245919 [details] the patch
Comment on attachment 245919 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=245919&action=review r=me with spelling fixes in ChangeLog. > Source/JavaScriptCore/ChangeLog:10 > + (JSC::DFG::Node::convertToMultiGetByOffset): Assert that we converted from something that alraedy had NodeMustGenerate. *already* > Source/JavaScriptCore/ChangeLog:11 > + * dfg/DFGNodeType.h: We shouldn't DCE a node that does checks and could be effectful in bseline. Making MultiGetBYoffset as NodeMustGenerate prevents DCE. FTL could still DCE the actual loads, but the checks will stay. *baseline*
(In reply to comment #2) > Comment on attachment 245919 [details] > the patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=245919&action=review > > r=me with spelling fixes in ChangeLog. > > > Source/JavaScriptCore/ChangeLog:10 > > + (JSC::DFG::Node::convertToMultiGetByOffset): Assert that we converted from something that alraedy had NodeMustGenerate. > > *already* > > > Source/JavaScriptCore/ChangeLog:11 > > + * dfg/DFGNodeType.h: We shouldn't DCE a node that does checks and could be effectful in bseline. Making MultiGetBYoffset as NodeMustGenerate prevents DCE. FTL could still DCE the actual loads, but the checks will stay. > > *baseline* Thanks! Fixed both locally.
Landed in http://trac.webkit.org/changeset/179536