Patch forthcoming.
Created attachment 220550 [details] the patch
Comment on attachment 220550 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=220550&action=review r+ with comments. > Source/JavaScriptCore/ChangeLog:10 > + was the only exception to that rule, and that was one of the reasons why we had this bug. Provide a description of what you did. > Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:1558 > + Node* charCode = addToGraph(StringCharCodeAt, OpInfo(ArrayMode(Array::String).asWord()), get(VirtualRegister(thisOperand)), get(indexOperand)); Why the VirtualRegister(thisOperand)? Use thisOperand directly. > Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:1570 > + Node* charCode = addToGraph(StringCharAt, OpInfo(ArrayMode(Array::String).asWord()), get(VirtualRegister(thisOperand)), get(indexOperand)); Why the VirtualRegister(thisOperand)? Use thisOperand directly. > Source/JavaScriptCore/dfg/DFGFixupPhase.cpp:1618 > + fixIntEdge(m_currentNode->child1()) | fixIntEdge(m_currentNode->child2()); You really want a binary OR (|) and not a logical OR (||)?
(In reply to comment #2) > (From update of attachment 220550 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=220550&action=review > > r+ with comments. > > > Source/JavaScriptCore/ChangeLog:10 > > + was the only exception to that rule, and that was one of the reasons why we had this bug. > > Provide a description of what you did. OK! > > > Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:1558 > > + Node* charCode = addToGraph(StringCharCodeAt, OpInfo(ArrayMode(Array::String).asWord()), get(VirtualRegister(thisOperand)), get(indexOperand)); > > Why the VirtualRegister(thisOperand)? Use thisOperand directly. Oops. > > > Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:1570 > > + Node* charCode = addToGraph(StringCharAt, OpInfo(ArrayMode(Array::String).asWord()), get(VirtualRegister(thisOperand)), get(indexOperand)); > > Why the VirtualRegister(thisOperand)? Use thisOperand directly. Oops. > > > Source/JavaScriptCore/dfg/DFGFixupPhase.cpp:1618 > > + fixIntEdge(m_currentNode->child1()) | fixIntEdge(m_currentNode->child2()); > > You really want a binary OR (|) and not a logical OR (||)? Yes. I want to call fixIntEdge() on both edges. And then if either one of those calls returns true, I want to do the Phantom thing.
(In reply to comment #3) > (In reply to comment #2) > > (From update of attachment 220550 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=220550&action=review > > > Source/JavaScriptCore/dfg/DFGFixupPhase.cpp:1618 > > > + fixIntEdge(m_currentNode->child1()) | fixIntEdge(m_currentNode->child2()); > > > > You really want a binary OR (|) and not a logical OR (||)? > > Yes. I want to call fixIntEdge() on both edges. And then if either one of those calls returns true, I want to do the Phantom thing. Okay. Then it may make sense to put a comment so someone doesn't come along and ruin things by turning it into a "||". Something about making sure both calls are made instead of the compiler optimizing out the second call.
(In reply to comment #4) > (In reply to comment #3) > > (In reply to comment #2) > > > (From update of attachment 220550 [details] [details] [details]) > > > View in context: https://bugs.webkit.org/attachment.cgi?id=220550&action=review > > > > > Source/JavaScriptCore/dfg/DFGFixupPhase.cpp:1618 > > > > + fixIntEdge(m_currentNode->child1()) | fixIntEdge(m_currentNode->child2()); > > > > > > You really want a binary OR (|) and not a logical OR (||)? > > > > Yes. I want to call fixIntEdge() on both edges. And then if either one of those calls returns true, I want to do the Phantom thing. > > Okay. Then it may make sense to put a comment so someone doesn't come along and ruin things by turning it into a "||". Something about making sure both calls are made instead of the compiler optimizing out the second call. Added.
Landed in http://trac.webkit.org/changeset/161465