[Crankshaft] Fine tune merging of Ignition and FCG feedback for binary/compare ops.

src/type-info.cc

Index: src/type-info.cc
diff --git a/src/type-info.cc b/src/type-info.cc
index efbc9b1f0759923717ee551cc98ae99328bfb4e3..59388cd2394ee2876b32bb90ef71b4a08425939b 100644
--- a/src/type-info.cc
+++ b/src/type-info.cc
@@ -220,9 +220,8 @@ AstType* BinaryOpFeedbackToType(int hint) {
       return AstType::Number();
     case BinaryOperationFeedback::kString:
       return AstType::String();
-    // TODO(mythria): Merge Number and NumberOrOddball feedback, after
-    // fixing crankshaft to handle Oddballs along with Numbers.
     case BinaryOperationFeedback::kNumberOrOddball:
+      return AstType::NumberOrOddball();
     case BinaryOperationFeedback::kAny:
     default:
       return AstType::Any();
@@ -264,14 +263,33 @@ void TypeFeedbackOracle::CompareType(TypeFeedbackId id, FeedbackVectorSlot slot,
     CompareICStub stub(code->stub_key(), isolate());
     AstType* left_type_from_ic =
         CompareICState::StateToType(zone(), stub.left());
-    *left_type = AstType::Union(*left_type, left_type_from_ic, zone());
     AstType* right_type_from_ic =
         CompareICState::StateToType(zone(), stub.right());
-    *right_type = AstType::Union(*right_type, right_type_from_ic, zone());
     AstType* combined_type_from_ic =
         CompareICState::StateToType(zone(), stub.state(), map);
-    *combined_type =
-        AstType::Union(*combined_type, combined_type_from_ic, zone());
+    // Full-codegen collects lhs and rhs feedback seperately and crankshaft

[rmcilroy, 2016/11/10 08:45:27]

Capitalise Crankshaft, Full-codegen and Ignition (in all comments below)

[mythria, 2016/11/10 11:43:01]

Done.

+    // could use this information to optimize better. So if combining the
+    // feedback has made the feedback less precise, we can use the feedback

[rmcilroy, 2016/11/10 08:45:27]

Can use -> should use

[mythria, 2016/11/10 11:43:01]

Done.

+    // only from full-codegen. If the union of feedback from full-codegen and

[rmcilroy, 2016/11/10 08:45:27]

Of feedback -> of the feedback

[mythria, 2016/11/10 11:43:01]

Done.

+    // if it is same as that of ignition, there is no need of combining

[rmcilroy, 2016/11/10 08:45:27]

Of combining -> to combine

[rmcilroy, 2016/11/10 08:45:28]

and if it is same as -> is the same as

[mythria, 2016/11/10 11:43:01]

Done.

[mythria, 2016/11/10 11:43:01]

Done.

+    // feedback from ignition.
+    AstType* combined_type_from_fcg = AstType::Union(
+        left_type_from_ic,
+        AstType::Union(right_type_from_ic, combined_type_from_ic, zone()),
+        zone());
+    if (combined_type_from_fcg == *left_type) {

[mythria, 2016/11/09 16:18:49]

I am not sure if == is the right way to check equality of AstTypes or if it is
better to have (combined_type_from_fcg->Is(*left_type) &&
(*left_type)->Is(combined_type_from_fcg) is better.

+      // Just pass FCG feedback. FCG collects information about lhs, rhs and
+      // result types seperately. So just retain that FCG information.
+      *left_type = left_type_from_ic;
+      *right_type = right_type_from_ic;
+      *combined_type = combined_type_from_ic;
+    } else {
+      // Combine ignition and FCG feedbacks.
+      *left_type = AstType::Union(*left_type, left_type_from_ic, zone());
+      *right_type = AstType::Union(*right_type, right_type_from_ic, zone());
+      *combined_type =
+          AstType::Union(*combined_type, combined_type_from_ic, zone());
+    }
   }
 }
 
@@ -313,9 +331,29 @@ void TypeFeedbackOracle::BinaryType(TypeFeedbackId id, FeedbackVectorSlot slot,
   BinaryOpICState state(isolate(), code->extra_ic_state());
   DCHECK_EQ(op, state.op());
 
-  *left = AstType::Union(*left, state.GetLeftType(), zone());
-  *right = AstType::Union(*right, state.GetRightType(), zone());
-  *result = AstType::Union(*result, state.GetResultType(), zone());
+  // Full-codegen collects lhs and rhs feedback seperately and crankshaft
+  // could use this information to optimize better. So if combining the
+  // feedback has made the feedback less precise, we can use the feedback
+  // only from full-codegen. If the union of feedback from full-codegen and
+  // if it is same as that of ignition, there is no need of combining
+  // feedback from ignition.

[rmcilroy, 2016/11/10 08:45:28]

Same comments as above.

+  AstType* combined_type_from_fcg = AstType::Union(
+      state.GetLeftType(),
+      AstType::Union(state.GetRightType(), state.GetResultType(), zone()),
+      zone());
+  if (combined_type_from_fcg == *left) {
+    // Just pass FCG feedback. FCG collects information about lhs, rhs and
+    // result types seperately. So just retain that FCG information.
+    *left = state.GetLeftType();
+    *right = state.GetRightType();
+    *result = state.GetResultType();
+  } else {
+    // Combine ignition and FCG feedback.
+    *left = AstType::Union(*left, state.GetLeftType(), zone());
+    *right = AstType::Union(*right, state.GetRightType(), zone());
+    *result = AstType::Union(*result, state.GetResultType(), zone());
+  }
+  // Ignition does not collect this feedback.
   *fixed_right_arg = state.fixed_right_arg();
 
   AllocationSite* first_allocation_site = code->FindFirstAllocationSite();