...
Created attachment 396650 [details] Patch
Comment on attachment 396650 [details] Patch Please add DFG and FTL implementations.
(In reply to Yusuke Suzuki from comment #2) > Please add DFG and FTL implementations. I am probably missing something, but don't we already have those? SpeculativeJIT::compileSameValue() (at dfg/DFGSpeculativeJIT.cpp:6673) FTLLowerDFGToB3::compileSameValue() (at ftl/FTLLowerDFGToB3.cpp:8831)
(In reply to Alexey Shvayka from comment #3) > (In reply to Yusuke Suzuki from comment #2) > > Please add DFG and FTL implementations. > > I am probably missing something, but don't we already have those? > > SpeculativeJIT::compileSameValue() (at dfg/DFGSpeculativeJIT.cpp:6673) > FTLLowerDFGToB3::compileSameValue() (at ftl/FTLLowerDFGToB3.cpp:8831) No, this is implementation of SameValue DFG node. But DFGByteCodeParser is not handling op_same_value to generate this node. I think we can see crash if we compile the code including op_same_value with DFG. Can you add a test for this?
Created attachment 396708 [details] Patch Add test, handle op_same_value in DFGByteCodeParser, add missing baseline JIT ops definitions.
Comment on attachment 396708 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=396708&action=review > Source/JavaScriptCore/ChangeLog:13 > + Int32 and String comparisons, which are exempt of handling NaN and negative zero values. I think we should implement @sameValue as a function call instead of bytecode. Let's define sameValue function like this, 1. Define link-time-constant @sameValue: See m_arrayProtoValuesFunction case for example. Define `m_linkTimeConstants[static_cast<unsigned>(LinkTimeConstant::sameValue)].initLater(...)`. 2. Initialize this with Object.is definition: Performing putWithoutTransition with LinkTimeConstant::sameValue's value. 3. Put this function as `Object.is` in ObjectConstructor 4. Call this @sameValue from builtins. So, then, DFG/FTL handles it as SameValue DFG node if this function has SameValue intrinsic appropriately.
Created attachment 396821 [details] Patch Expose Object.is function as @sameValue.
(In reply to Yusuke Suzuki from comment #6) > I think we should implement @sameValue as a function call instead of > bytecode. > Let's define sameValue function like this, I am not sure I've strictly followed your instructions. I didn't manage to find any precedent of a method put to a prototype/constructor from LinkTimeConstant, yet I've seen quite a few prototype/constructor methods that were put to LinkTimeConstant (like `thisTimeValue`). Also it would be nice to keep everything in `objectConstructorTable` in case we would want to reorder methods per spec.
Comment on attachment 396821 [details] Patch r=me. I don't think putting `is` in ObjectConstructor table is important compared to allocating one more object since it requires memory, but maybe both does not matter much.
Committed r260312: <https://trac.webkit.org/changeset/260312> All reviewed patches have been landed. Closing bug and clearing flags on attachment 396821 [details].
<rdar://problem/61973276>
(In reply to Yusuke Suzuki from comment #9) > Comment on attachment 396821 [details] > Patch > > r=me. I don't think putting `is` in ObjectConstructor table is important > compared to allocating one more object since it requires memory, but maybe > both does not matter much. Apart from Object.is, there are a few link-time-constants that duplicate built-in methods: * Date.prototype.{valueOf,getTime} == LinkTimeConstant::thisTimeValue * Math.trunc == LinkTimeConstant::trunc * String.prototype.repeat == LinkTimeConstant::repeatCharacter I've tried adding ``` putDirectWithoutTransition(vm, Identifier::fromString(vm, "is"), globalObject->linkTimeConstant(LinkTimeConstant::sameValue), static_cast<unsigned>(PropertyAttribute::DontEnum)); ``` to ObjectConstructor::finishCreation(), but it looks it runs before link-time-constant is initialized: .../WebKit/Source/JavaScriptCore/runtime/JSGlobalObject.h(917) : JSC::JSCell *JSC::JSGlobalObject::linkTimeConstant(JSC::LinkTimeConstant) const 1 0x10acff479 WTFCrash 2 0x10b4c565b WTFCrashWithInfo(int, char const*, char const*, int) 3 0x10b78d2ff JSC::JSGlobalObject::linkTimeConstant(JSC::LinkTimeConstant) const 4 0x10c6f08cb JSC::ObjectConstructor::finishCreation(JSC::VM&, JSC::JSGlobalObject*, JSC::ObjectPrototype*) 5 0x10c5ac80f JSC::ObjectConstructor::create(JSC::VM&, JSC::JSGlobalObject*, JSC::Structure*, JSC::ObjectPrototype*) 6 0x10c5a29ba JSC::JSGlobalObject::init(JSC::VM&) 7 0x10c5b8804 JSC::JSGlobalObject::finishCreation(JSC::VM&) 8 0x107616a01 GlobalObject::finishCreation(JSC::VM&, WTF::Vector<WTF::String, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc> const&) 9 0x1076154e7 GlobalObject::create(JSC::VM&, JSC::Structure*, WTF::Vector<WTF::String, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc> const&) 10 0x1075f1241 int runJSC<jscmain(int, char**)::$_6>(CommandLine const&, bool, jscmain(int, char**)::$_6 const&) 11 0x1075efc58 jscmain(int, char**) 12 0x1075ef9be main 13 0x7fff70ef7cc9 start 14 0x2 No luck with Date.prototype and String.prototype either. Math.trunc works though.