Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(2470)

Unified Diff: runtime/vm/intermediate_language_dbc.cc

Issue 2937933002: Reduce copying, redundancy & repetition for codegen of comparison instructions (Closed)
Patch Set: Created 3 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: runtime/vm/intermediate_language_dbc.cc
diff --git a/runtime/vm/intermediate_language_dbc.cc b/runtime/vm/intermediate_language_dbc.cc
index ea9c9d8018624e0de891a80c48c28b431b037863..519c8fc4717f908854acad07472c1ca028e95d76 100644
--- a/runtime/vm/intermediate_language_dbc.cc
+++ b/runtime/vm/intermediate_language_dbc.cc
@@ -483,6 +483,13 @@ static void EmitBranchOnCondition(FlowGraphCompiler* compiler,
}
+Condition StrictCompareInstr::GetPredictedCondition(FlowGraphCompiler* compiler,
Vyacheslav Egorov (Google) 2017/06/15 11:32:46 The name GetPredicatedCondition(...) is very confu
erikcorry 2017/06/19 07:15:09 Done.
+ BranchLabels labels) {
+ return (labels.fall_through == labels.false_label) ? NEXT_IS_TRUE
+ : NEXT_IS_FALSE;
+}
+
+
Condition StrictCompareInstr::EmitComparisonCode(FlowGraphCompiler* compiler,
BranchLabels labels) {
ASSERT((kind() == Token::kNE_STRICT) || (kind() == Token::kEQ_STRICT));
@@ -526,47 +533,91 @@ Condition StrictCompareInstr::EmitComparisonCode(FlowGraphCompiler* compiler,
}
-void StrictCompareInstr::EmitBranchCode(FlowGraphCompiler* compiler,
- BranchInstr* branch) {
- ASSERT((kind() == Token::kEQ_STRICT) || (kind() == Token::kNE_STRICT));
+DEFINE_MAKE_LOCATION_SUMMARY(StrictCompare,
+ 2,
+ Location::RequiresRegister(),
+ needs_number_check() ? LocationSummary::kCall
+ : LocationSummary::kNoCall)
+
+template <intptr_t N,
+ typename ThrowsTrait,
+ template <typename Impure, typename Pure> class CSETrait>
+void TemplateComparison<N, ThrowsTrait, CSETrait>::EmitBranchCode(
+ FlowGraphCompiler* compiler,
+ BranchInstr* branch) {
BranchLabels labels = compiler->CreateBranchLabels(branch);
Condition true_condition = EmitComparisonCode(compiler, labels);
- EmitBranchOnCondition(compiler, true_condition, labels);
+ if (true_condition != INVALID_CONDITION) {
+ EmitBranchOnCondition(compiler, true_condition, labels);
+ }
}
-EMIT_NATIVE_CODE(StrictCompare,
- 2,
- Location::RequiresRegister(),
- needs_number_check() ? LocationSummary::kCall
- : LocationSummary::kNoCall) {
- ASSERT((kind() == Token::kEQ_STRICT) || (kind() == Token::kNE_STRICT));
+// IA32 compiler needs this to link.
+template <>
+void TemplateComparison<2, Throws, NoCSE>::EmitBranchCode(
+ FlowGraphCompiler* compiler,
+ BranchInstr* branch) {
+ UNREACHABLE();
+}
+
+template <intptr_t N,
+ typename ThrowsTrait,
+ template <typename Impure, typename Pure> class CSETrait>
+void TemplateComparison<N, ThrowsTrait, CSETrait>::EmitNativeCode(
+ FlowGraphCompiler* compiler) {
Label is_true, is_false;
BranchLabels labels = {&is_true, &is_false, &is_false};
- Condition true_condition = EmitComparisonCode(compiler, labels);
- EmitBranchOnCondition(compiler, true_condition, labels);
- Label done;
- if (compiler->is_optimizing()) {
- const Register result = locs()->out(0).reg();
- __ Bind(&is_false);
- __ LoadConstant(result, Bool::False());
- __ Jump(&done);
- __ Bind(&is_true);
- __ LoadConstant(result, Bool::True());
- __ Bind(&done);
- } else {
+ Condition true_condition = this->GetPredictedCondition(compiler, labels);
+ if (true_condition == INVALID_CONDITION || !compiler->is_optimizing() ||
+ is_true.IsLinked() || is_false.IsLinked()) {
+ Condition actual_condition = EmitComparisonCode(compiler, labels);
+ ASSERT(actual_condition == true_condition);
+ if (true_condition != INVALID_CONDITION) {
+ EmitBranchOnCondition(compiler, true_condition, labels);
+ }
+ Label done;
__ Bind(&is_false);
__ PushConstant(Bool::False());
__ Jump(&done);
__ Bind(&is_true);
__ PushConstant(Bool::True());
__ Bind(&done);
+ } else {
+ const Register result = this->locs()->out(0).reg();
+ __ LoadConstant(
+ result, true_condition == NEXT_IS_TRUE ? Bool::False() : Bool::True());
+ Condition actual_condition = EmitComparisonCode(compiler, labels);
+ ASSERT(actual_condition == true_condition);
+ __ LoadConstant(
+ result, true_condition == NEXT_IS_TRUE ? Bool::True() : Bool::False());
}
}
+// IA32 compiler needs this to link (for the simmips config).
+template <>
+void TemplateComparison<2, Throws, NoCSE>::EmitNativeCode(
+ FlowGraphCompiler* compiler) {
+ UNREACHABLE();
+}
+
+
+// Explicit template instantiations.
+template void TemplateComparison<1, NoThrow, Pure>::EmitNativeCode(
+ FlowGraphCompiler*);
+template void TemplateComparison<2, NoThrow, Pure>::EmitNativeCode(
+ FlowGraphCompiler*);
+template void TemplateComparison<1, NoThrow, Pure>::EmitBranchCode(
+ FlowGraphCompiler* compiler,
+ BranchInstr* branch);
+template void TemplateComparison<2, NoThrow, Pure>::EmitBranchCode(
+ FlowGraphCompiler* compiler,
+ BranchInstr* branch);
+
+
LocationSummary* BranchInstr::MakeLocationSummary(Zone* zone, bool opt) const {
comparison()->InitializeLocationSummary(zone, opt);
if (!comparison()->HasLocs()) {
@@ -601,6 +652,13 @@ EMIT_NATIVE_CODE(Goto, 0) {
}
+Condition TestSmiInstr::GetPredictedCondition(FlowGraphCompiler* compiler,
+ BranchLabels labels) {
+ ASSERT((kind() == Token::kEQ) || (kind() == Token::kNE));
+ return (kind() == Token::kEQ) ? NEXT_IS_TRUE : NEXT_IS_FALSE;
+}
+
+
Condition TestSmiInstr::EmitComparisonCode(FlowGraphCompiler* compiler,
BranchLabels labels) {
ASSERT((kind() == Token::kEQ) || (kind() == Token::kNE));
@@ -611,21 +669,10 @@ Condition TestSmiInstr::EmitComparisonCode(FlowGraphCompiler* compiler,
}
-void TestSmiInstr::EmitBranchCode(FlowGraphCompiler* compiler,
- BranchInstr* branch) {
- BranchLabels labels = compiler->CreateBranchLabels(branch);
- Condition true_condition = EmitComparisonCode(compiler, labels);
- EmitBranchOnCondition(compiler, true_condition, labels);
-}
-
-
-EMIT_NATIVE_CODE(TestSmi,
- 2,
- Location::RequiresRegister(),
- LocationSummary::kNoCall) {
- // Never emitted outside of the BranchInstr.
- UNREACHABLE();
-}
+DEFINE_MAKE_LOCATION_SUMMARY(TestSmi,
+ 2,
+ Location::RequiresRegister(),
+ LocationSummary::kNoCall)
Condition TestCidsInstr::EmitComparisonCode(FlowGraphCompiler* compiler,
@@ -662,30 +709,16 @@ Condition TestCidsInstr::EmitComparisonCode(FlowGraphCompiler* compiler,
}
-void TestCidsInstr::EmitBranchCode(FlowGraphCompiler* compiler,
- BranchInstr* branch) {
- BranchLabels labels = compiler->CreateBranchLabels(branch);
- Condition true_condition = EmitComparisonCode(compiler, labels);
- EmitBranchOnCondition(compiler, true_condition, labels);
+Condition TestCidsInstr::GetPredictedCondition(FlowGraphCompiler* compiler,
+ BranchLabels labels) {
+ return NEXT_IS_TRUE;
}
-EMIT_NATIVE_CODE(TestCids,
- 1,
- Location::RequiresRegister(),
- LocationSummary::kNoCall) {
- Register result_reg = locs()->out(0).reg();
- Label is_true, is_false, done;
- BranchLabels labels = {&is_true, &is_false, &is_false};
- EmitComparisonCode(compiler, labels);
- __ Jump(&is_true);
- __ Bind(&is_false);
- __ LoadConstant(result_reg, Bool::False());
- __ Jump(&done);
- __ Bind(&is_true);
- __ LoadConstant(result_reg, Bool::True());
- __ Bind(&done);
-}
+DEFINE_MAKE_LOCATION_SUMMARY(TestCids,
+ 1,
+ Location::RequiresRegister(),
+ LocationSummary::kNoCall)
EMIT_NATIVE_CODE(CreateArray,
@@ -1753,15 +1786,7 @@ EMIT_NATIVE_CODE(BinaryDoubleOp, 2, Location::RequiresRegister()) {
Condition DoubleTestOpInstr::EmitComparisonCode(FlowGraphCompiler* compiler,
BranchLabels labels) {
- UNREACHABLE();
- return Condition();
-}
-
-
-void DoubleTestOpInstr::EmitBranchCode(FlowGraphCompiler* compiler,
- BranchInstr* branch) {
ASSERT(compiler->is_optimizing());
- BranchLabels labels = compiler->CreateBranchLabels(branch);
const Register value = locs()->in(0).reg();
switch (op_kind()) {
case MethodRecognizer::kDouble_getIsNaN:
@@ -1774,31 +1799,20 @@ void DoubleTestOpInstr::EmitBranchCode(FlowGraphCompiler* compiler,
UNREACHABLE();
}
const bool is_negated = kind() != Token::kEQ;
Vyacheslav Egorov (Google) 2017/06/15 11:32:46 return GetPredictedCondition();
erikcorry 2017/06/19 07:15:09 See above
- EmitBranchOnCondition(compiler, is_negated ? NEXT_IS_FALSE : NEXT_IS_TRUE,
- labels);
+ return is_negated ? NEXT_IS_FALSE : NEXT_IS_TRUE;
}
-EMIT_NATIVE_CODE(DoubleTestOp, 1, Location::RequiresRegister()) {
- ASSERT(compiler->is_optimizing());
- const Register value = locs()->in(0).reg();
- const Register result = locs()->out(0).reg();
+Condition DoubleTestOpInstr::GetPredictedCondition(FlowGraphCompiler* compiler,
+ BranchLabels labels) {
const bool is_negated = kind() != Token::kEQ;
- __ LoadConstant(result, is_negated ? Bool::True() : Bool::False());
- switch (op_kind()) {
- case MethodRecognizer::kDouble_getIsNaN:
- __ DoubleIsNaN(value);
- break;
- case MethodRecognizer::kDouble_getIsInfinite:
- __ DoubleIsInfinite(value);
- break;
- default:
- UNREACHABLE();
- }
- __ LoadConstant(result, is_negated ? Bool::False() : Bool::True());
+ return is_negated ? NEXT_IS_FALSE : NEXT_IS_TRUE;
}
+DEFINE_MAKE_LOCATION_SUMMARY(DoubleTestOp, 1, Location::RequiresRegister())
+
+
EMIT_NATIVE_CODE(UnaryDoubleOp, 1, Location::RequiresRegister()) {
const Register value = locs()->in(0).reg();
const Register result = locs()->out(0).reg();
@@ -1973,7 +1987,8 @@ static Condition EmitSmiComparisonOp(FlowGraphCompiler* compiler,
// If we aren't falling through to the false label, we can save a Jump
// instruction in the case that the true case is the fall through by
// flipping the sense of the test such that the instruction following the
- // test is the Jump to the false label.
+ // test is the Jump to the false label. In the case where both labels are
+ // null we don't flip the sense of the test.
condition = NEXT_IS_FALSE;
comparison = FlipCondition(kind);
}
@@ -2010,29 +2025,20 @@ Condition EqualityCompareInstr::EmitComparisonCode(FlowGraphCompiler* compiler,
}
-EMIT_NATIVE_CODE(EqualityCompare, 2, Location::RequiresRegister()) {
- ASSERT(compiler->is_optimizing());
- ASSERT((kind() == Token::kEQ) || (kind() == Token::kNE));
- Label is_true, is_false;
- // These labels are not used. They are arranged so that EmitComparisonCode
- // emits a test that executes the following instruction when the test
- // succeeds.
- BranchLabels labels = {&is_true, &is_false, &is_false};
- const Register result = locs()->out(0).reg();
- __ LoadConstant(result, Bool::False());
- Condition true_condition = EmitComparisonCode(compiler, labels);
- ASSERT(true_condition == NEXT_IS_TRUE);
- __ LoadConstant(result, Bool::True());
+Condition EqualityCompareInstr::GetPredictedCondition(
+ FlowGraphCompiler* compiler,
+ BranchLabels labels) {
+ if (operation_cid() == kSmiCid) {
+ return (labels.fall_through != labels.false_label) ? NEXT_IS_FALSE
+ : NEXT_IS_TRUE;
+ } else {
+ ASSERT(operation_cid() == kDoubleCid);
+ return NEXT_IS_TRUE;
+ }
}
-void EqualityCompareInstr::EmitBranchCode(FlowGraphCompiler* compiler,
- BranchInstr* branch) {
- ASSERT((kind() == Token::kNE) || (kind() == Token::kEQ));
- BranchLabels labels = compiler->CreateBranchLabels(branch);
- Condition true_condition = EmitComparisonCode(compiler, labels);
- EmitBranchOnCondition(compiler, true_condition, labels);
-}
+DEFINE_MAKE_LOCATION_SUMMARY(EqualityCompare, 2, Location::RequiresRegister());
Condition RelationalOpInstr::EmitComparisonCode(FlowGraphCompiler* compiler,
@@ -2046,24 +2052,19 @@ Condition RelationalOpInstr::EmitComparisonCode(FlowGraphCompiler* compiler,
}
-EMIT_NATIVE_CODE(RelationalOp, 2, Location::RequiresRegister()) {
- ASSERT(compiler->is_optimizing());
- Label is_true, is_false;
- BranchLabels labels = {&is_true, &is_false, &is_false};
- const Register result = locs()->out(0).reg();
- __ LoadConstant(result, Bool::False());
- Condition true_condition = EmitComparisonCode(compiler, labels);
- ASSERT(true_condition == NEXT_IS_TRUE);
- __ LoadConstant(result, Bool::True());
+Condition RelationalOpInstr::GetPredictedCondition(FlowGraphCompiler* compiler,
+ BranchLabels labels) {
+ if (operation_cid() == kSmiCid) {
+ return (labels.fall_through != labels.false_label) ? NEXT_IS_FALSE
+ : NEXT_IS_TRUE;
+ } else {
+ ASSERT(operation_cid() == kDoubleCid);
+ return NEXT_IS_TRUE;
+ }
}
-void RelationalOpInstr::EmitBranchCode(FlowGraphCompiler* compiler,
- BranchInstr* branch) {
- BranchLabels labels = compiler->CreateBranchLabels(branch);
- Condition true_condition = EmitComparisonCode(compiler, labels);
- EmitBranchOnCondition(compiler, true_condition, labels);
-}
+DEFINE_MAKE_LOCATION_SUMMARY(RelationalOp, 2, Location::RequiresRegister())
EMIT_NATIVE_CODE(CheckArrayBound, 2) {

Powered by Google App Engine
This is Rietveld 408576698