Patch forthcoming.
Created attachment 283485 [details] the patch
Comment on attachment 283485 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=283485&action=review r=me with question. > Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:8268 > + m_jit.boxBooleanPayload(false, resultGPR); > + JITCompiler::JumpList notEqual = m_jit.branchIfNotEqual(regs, node->cellOperand()->value()); > + m_jit.boxBooleanPayload(true, resultGPR); > + notEqual.link(&m_jit); Is this more efficient than doing: JITCompiler::JumpList notEqual = m_jit.branchIfNotEqual(regs, node->cellOperand()->value()); m_jit.boxBooleanPayload(true, resultGPR); JITCompiler::Jump done = m_jit.jump(); notEqual.link(&m_jit); m_jit.boxBooleanPayload(false, resultGPR); done.link(&m_jit);
(In reply to comment #2) > Comment on attachment 283485 [details] > the patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=283485&action=review > > r=me with question. > > > Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:8268 > > + m_jit.boxBooleanPayload(false, resultGPR); > > + JITCompiler::JumpList notEqual = m_jit.branchIfNotEqual(regs, node->cellOperand()->value()); > > + m_jit.boxBooleanPayload(true, resultGPR); > > + notEqual.link(&m_jit); > > Is this more efficient than doing: > > JITCompiler::JumpList notEqual = m_jit.branchIfNotEqual(regs, > node->cellOperand()->value()); > m_jit.boxBooleanPayload(true, resultGPR); > JITCompiler::Jump done = m_jit.jump(); > notEqual.link(&m_jit); > m_jit.boxBooleanPayload(false, resultGPR); > done.link(&m_jit); I don't know! I think that my version is better because: - Moving zero into a register is basically free. - Your version will do two branches in the worst case, while mine will do one. One of your branches is a jump, which is cheap, but probably worse than zeroing a register. My version also involves less code. I like to err on the side of small code when everything else is up in the air.
(In reply to comment #3) > (In reply to comment #2) > > Comment on attachment 283485 [details] > > the patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=283485&action=review > > > > r=me with question. > > > > > Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:8268 > > > + m_jit.boxBooleanPayload(false, resultGPR); > > > + JITCompiler::JumpList notEqual = m_jit.branchIfNotEqual(regs, node->cellOperand()->value()); > > > + m_jit.boxBooleanPayload(true, resultGPR); > > > + notEqual.link(&m_jit); > > > > Is this more efficient than doing: > > > > JITCompiler::JumpList notEqual = m_jit.branchIfNotEqual(regs, > > node->cellOperand()->value()); > > m_jit.boxBooleanPayload(true, resultGPR); > > JITCompiler::Jump done = m_jit.jump(); > > notEqual.link(&m_jit); > > m_jit.boxBooleanPayload(false, resultGPR); > > done.link(&m_jit); > > I don't know! I think that my version is better because: > > - Moving zero into a register is basically free. > - Your version will do two branches in the worst case, while mine will do > one. One of your branches is a jump, which is cheap, but probably worse > than zeroing a register. > > My version also involves less code. I like to err on the side of small code > when everything else is up in the air. Brainfart: I'm not moving zero into a register. I guess the question is: do we care about this code being fast or small? I like small a bit better, since if it really needs to be fast, it'll get to FTL. DFG is for functions that don't run that much, so icache matters more.
Created attachment 283486 [details] patch for landing
Created attachment 283549 [details] performance Looks pretty good!
Landed in https://trac.webkit.org/changeset/203361