This function was somewhat hot inside the ML subtest in ARES. Making it an intrinsic is not a perf improvement as far as I can measure, but I already did the work, so we might as well land it, since it is a perf improvement over calling the builtin function.
Created attachment 334545 [details] patch
Comment on attachment 334545 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=334545&action=review r=me, a few comments. > JSTests/stress/number-is-integer-intrinsic.js:45 > + [function(){}, false], Add something with valueOf (returning both true and false things). > Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp:4670 > + auto notNanOrInfinity = m_jit.branch32(JITCompiler::NotEqual, TrustedImm32(0x7ff), resultGPR); notNanNorInfinity? > Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp:4677 > + m_jit.or32(TrustedImm32(ValueFalse), resultGPR); Instead of having notNaNNorInfinity jump above, can you invert it, fall through where it should, and jump here if fails? Then you only OR in ValueFalse in one place (instead of move above, and moving ValueTrue below). I think it works if you set resultGPR early.
Comment on attachment 334545 [details] patch Attachment 334545 [details] did not pass jsc-ews (mac): Output: http://webkit-queues.webkit.org/results/6645323 New failing tests: stress/number-is-integer-intrinsic.js.ftl-eager-no-cjit stress/number-is-integer-intrinsic.js.ftl-no-cjit-no-put-stack-validate stress/number-is-integer-intrinsic.js.ftl-no-cjit-no-inline-validate stress/number-is-integer-intrinsic.js.ftl-eager-no-cjit-b3o1 stress/number-is-integer-intrinsic.js.ftl-no-cjit-validate-sampling-profiler
Comment on attachment 334545 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=334545&action=review >> JSTests/stress/number-is-integer-intrinsic.js:45 >> + [function(){}, false], > > Add something with valueOf (returning both true and false things). Thankfully, valueOf does not get called for isInteger. >> Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp:4670 >> + auto notNanOrInfinity = m_jit.branch32(JITCompiler::NotEqual, TrustedImm32(0x7ff), resultGPR); > > notNanNorInfinity? Yeah, will fix. >> Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp:4677 >> + m_jit.or32(TrustedImm32(ValueFalse), resultGPR); > > Instead of having notNaNNorInfinity jump above, can you invert it, fall through where it should, and jump here if fails? Then you only OR in ValueFalse in one place (instead of move above, and moving ValueTrue below). I think it works if you set resultGPR early. Yeah, this could work, but only if I do not mark result as "Reuse" above. The Reuse above makes it so that valueRegs.gpr() and resultGPR may be the same register. If they were, we couldn't do this. I went with trying to have less register pressure than this approach, but who knows if that's the better decision. I'm not really sure.
Created attachment 334549 [details] patch for landing
Comment on attachment 334549 [details] patch for landing Clearing flags on attachment: 334549 Committed r228968: <https://trac.webkit.org/changeset/228968>
All reviewed patches have been landed. Closing bug.
<rdar://problem/37843990>