WebKit Bugzilla
Attachment 357918 Details for
Bug 192966
: DFGByteCodeParser rules for bitwise operations should consider type of their operands
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
bug-192966-20181221012006.patch (text/plain), 9.04 KB, created by
Caio Lima
on 2018-12-20 19:20:08 PST
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
Caio Lima
Created:
2018-12-20 19:20:08 PST
Size:
9.04 KB
patch
obsolete
>Subversion Revision: 239478 >diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog >index 5f116701d091550e6c534081df0c71a5382cc584..c64dde4afc1a6345ace56599d8f4e59b906c91ed 100644 >--- a/Source/JavaScriptCore/ChangeLog >+++ b/Source/JavaScriptCore/ChangeLog >@@ -1,3 +1,30 @@ >+2018-12-20 Caio Lima <ticaiolima@gmail.com> >+ >+ DFGByteCodeParser rules for bitwise operations should consider type of their operands >+ https://bugs.webkit.org/show_bug.cgi?id=192966 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ This patch is changing the logic how we lower bitwise operations, to >+ consider only the type of input nodes and fix them during FixupPhase, >+ if necessary. We are also changing the prediction propagation rules >+ for ValueBitOp, since they can produce better propagation when we >+ consider their operands prediction. This change will remove the >+ necessity of execution bitwise operations to properly emit AirthBitOp >+ becaue we are not relying into `getPrediction()` call anymore. >+ >+ * dfg/DFGBackwardsPropagationPhase.cpp: >+ (JSC::DFG::BackwardsPropagationPhase::isWithinPowerOfTwo): >+ (JSC::DFG::BackwardsPropagationPhase::propagate): >+ * dfg/DFGByteCodeParser.cpp: >+ (JSC::DFG::ByteCodeParser::parseBlock): >+ * dfg/DFGFixupPhase.cpp: >+ (JSC::DFG::FixupPhase::fixupNode): >+ * dfg/DFGNode.h: >+ (JSC::DFG::Node::hasInt32Result): >+ (JSC::DFG::Node::hasNumberOrAnyIntResult): >+ * dfg/DFGPredictionPropagationPhase.cpp: >+ > 2018-12-20 Chris Dumez <cdumez@apple.com> > > Use Optional::hasValue() instead of Optional::has_value() >diff --git a/Source/JavaScriptCore/dfg/DFGBackwardsPropagationPhase.cpp b/Source/JavaScriptCore/dfg/DFGBackwardsPropagationPhase.cpp >index 05c86ed1eb05f129ed4d00206d31e4e1f25d6159..f4ce71eba58adbd5b36109e6fb6b32962cdd72f9 100644 >--- a/Source/JavaScriptCore/dfg/DFGBackwardsPropagationPhase.cpp >+++ b/Source/JavaScriptCore/dfg/DFGBackwardsPropagationPhase.cpp >@@ -110,6 +110,7 @@ private: > return isWithinPowerOfTwoForConstant<power>(node); > } > >+ case ValueBitAnd: > case ArithBitAnd: { > if (power > 31) > return true; >@@ -120,6 +121,8 @@ private: > > case ArithBitOr: > case ArithBitXor: >+ case ValueBitOr: >+ case ValueBitXor: > case BitLShift: { > return power > 31; > } >@@ -218,6 +221,9 @@ private: > case ArithBitAnd: > case ArithBitOr: > case ArithBitXor: >+ case ValueBitAnd: >+ case ValueBitOr: >+ case ValueBitXor: > case BitRShift: > case BitLShift: > case BitURShift: >diff --git a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp >index d6e3843a8d8349490154e08777274f08655f650b..337396ae62a09ba2f05344b2ada07212a2acda30 100644 >--- a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp >+++ b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp >@@ -4934,7 +4934,7 @@ void ByteCodeParser::parseBlock(unsigned limit) > auto bytecode = currentInstruction->as<OpBitand>(); > Node* op1 = get(bytecode.lhs); > Node* op2 = get(bytecode.rhs); >- if (isInt32Speculation(getPrediction())) >+ if (op1->hasNumberOrAnyIntResult() && op2->hasNumberOrAnyIntResult()) > set(bytecode.dst, addToGraph(ArithBitAnd, op1, op2)); > else > set(bytecode.dst, addToGraph(ValueBitAnd, op1, op2)); >@@ -4945,7 +4945,7 @@ void ByteCodeParser::parseBlock(unsigned limit) > auto bytecode = currentInstruction->as<OpBitor>(); > Node* op1 = get(bytecode.lhs); > Node* op2 = get(bytecode.rhs); >- if (isInt32Speculation(getPrediction())) >+ if (op1->hasNumberOrAnyIntResult() && op2->hasNumberOrAnyIntResult()) > set(bytecode.dst, addToGraph(ArithBitOr, op1, op2)); > else > set(bytecode.dst, addToGraph(ValueBitOr, op1, op2)); >@@ -4956,7 +4956,7 @@ void ByteCodeParser::parseBlock(unsigned limit) > auto bytecode = currentInstruction->as<OpBitxor>(); > Node* op1 = get(bytecode.lhs); > Node* op2 = get(bytecode.rhs); >- if (isInt32Speculation(getPrediction())) >+ if (op1->hasNumberOrAnyIntResult() && op2->hasNumberOrAnyIntResult()) > set(bytecode.dst, addToGraph(ArithBitXor, op1, op2)); > else > set(bytecode.dst, addToGraph(ValueBitXor, op1, op2)); >diff --git a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp >index 307632dad59da8e23250db1b4497d0341ba95292..ed243bc8bc62331defd54492fd655f28ca8282f9 100644 >--- a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp >+++ b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp >@@ -206,8 +206,32 @@ private: > break; > } > >- fixEdge<UntypedUse>(node->child1()); >- fixEdge<UntypedUse>(node->child2()); >+ if (Node::shouldSpeculateUntypedForBitOps(node->child1().node(), node->child2().node())) { >+ fixEdge<UntypedUse>(node->child1()); >+ fixEdge<UntypedUse>(node->child2()); >+ break; >+ } >+ >+ // In such case, we need to fallback to ArithBitOp >+ switch (op) { >+ case ValueBitXor: >+ node->setOp(ArithBitXor); >+ break; >+ case ValueBitOr: >+ node->setOp(ArithBitOr); >+ break; >+ case ValueBitAnd: >+ node->setOp(ArithBitAnd); >+ break; >+ default: >+ DFG_CRASH(m_graph, node, "Unexpected node during ValueBit operation fixup"); >+ break; >+ } >+ >+ node->clearFlags(NodeMustGenerate); >+ node->setResult(NodeResultInt32); >+ fixIntConvertingEdge(node->child1()); >+ fixIntConvertingEdge(node->child2()); > break; > } > >@@ -225,26 +249,6 @@ private: > case ArithBitXor: > case ArithBitOr: > case ArithBitAnd: { >- if (Node::shouldSpeculateUntypedForBitOps(node->child1().node(), node->child2().node())) { >- fixEdge<UntypedUse>(node->child1()); >- fixEdge<UntypedUse>(node->child2()); >- switch (op) { >- case ArithBitXor: >- node->setOpAndDefaultFlags(ValueBitXor); >- break; >- case ArithBitOr: >- node->setOpAndDefaultFlags(ValueBitOr); >- break; >- case ArithBitAnd: >- node->setOpAndDefaultFlags(ValueBitAnd); >- break; >- default: >- DFG_CRASH(m_graph, node, "Unexpected node during ArithBit operation fixup"); >- break; >- } >- break; >- } >- > fixIntConvertingEdge(node->child1()); > fixIntConvertingEdge(node->child2()); > break; >diff --git a/Source/JavaScriptCore/dfg/DFGNode.h b/Source/JavaScriptCore/dfg/DFGNode.h >index b8014755c7bcb50914ed39d61a53d94aca86021e..3e9d16f2de880c6db7731a0b0e1405b72d6f43e8 100644 >--- a/Source/JavaScriptCore/dfg/DFGNode.h >+++ b/Source/JavaScriptCore/dfg/DFGNode.h >@@ -1363,6 +1363,11 @@ public: > return !!result(); > } > >+ bool hasInt32Result() >+ { >+ return result() == NodeResultInt32; >+ } >+ > bool hasInt52Result() > { > return result() == NodeResultInt52; >@@ -1372,6 +1377,11 @@ public: > { > return result() == NodeResultNumber; > } >+ >+ bool hasNumberOrAnyIntResult() >+ { >+ return hasNumberResult() || hasInt32Result() || hasInt52Result(); >+ } > > bool hasNumericResult() > { >diff --git a/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp b/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp >index b3c69bcf124727bfcc676b2afadab5a88c22f93a..8074bfe63ad000df5f11e2881a62b33c44343e54 100644 >--- a/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp >+++ b/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp >@@ -280,8 +280,17 @@ private: > case ValueBitXor: > case ValueBitOr: > case ValueBitAnd: { >- if (node->child1()->shouldSpeculateBigInt() && node->child2()->shouldSpeculateBigInt()) >- changed |= mergePrediction(SpecBigInt); >+ SpeculatedType left = node->child1()->prediction(); >+ SpeculatedType right = node->child2()->prediction(); >+ >+ if (left && right) { >+ if (isBigIntSpeculation(left) && isBigIntSpeculation(right)) >+ changed |= mergePrediction(SpecBigInt); >+ else if (isFullNumberOrBooleanSpeculationExpectingDefined(left) && isFullNumberOrBooleanSpeculationExpectingDefined(right)) >+ changed |= mergePrediction(SpecInt32Only); >+ else >+ changed |= mergePrediction(SpecBigInt | SpecInt32Only); >+ } > break; > } >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 192966
:
357918
|
357919
|
358784
|
358786
|
359115