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

src/type-info.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/type-info.h"
 
 #include "src/ast/ast.h"
 #include "src/code-stubs.h"
 #include "src/ic/ic.h"
 #include "src/ic/stub-cache.h"
@@ skipping 202 matching lines @@
 AstType* BinaryOpFeedbackToType(int hint) {
   switch (hint) {
     case BinaryOperationFeedback::kNone:
       return AstType::None();
     case BinaryOperationFeedback::kSignedSmall:
       return AstType::SignedSmall();
     case BinaryOperationFeedback::kNumber:
       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();
   }
   UNREACHABLE();
   return AstType::None();
 }
 
 }  // end anonymous namespace
 
@@ skipping 21 matching lines @@
   // Merge the feedback from full-codegen if available.
   Handle<Code> code = Handle<Code>::cast(info);
   Handle<Map> map;
   Map* raw_map = code->FindFirstMap();
   if (raw_map != NULL) Map::TryUpdate(handle(raw_map)).ToHandle(&map);
 
   if (code->is_compare_ic_stub()) {
     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());
+    }
   }
 }
 
 void TypeFeedbackOracle::BinaryType(TypeFeedbackId id, FeedbackVectorSlot slot,
                                     AstType** left, AstType** right,
                                     AstType** result,
                                     Maybe<int>* fixed_right_arg,
                                     Handle<AllocationSite>* allocation_site,
                                     Token::Value op) {
   Handle<Object> object = GetInfo(id);
@@ skipping 21 matching lines @@
   *allocation_site = Handle<AllocationSite>::null();
 
   if (!object->IsCode()) return;
 
   // Merge the feedback from full-codegen if available.
   Handle<Code> code = Handle<Code>::cast(object);
   DCHECK_EQ(Code::BINARY_OP_IC, code->kind());
   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();
   if (first_allocation_site != NULL) {
     *allocation_site = handle(first_allocation_site);
   } else {
     *allocation_site = Handle<AllocationSite>::null();
   }
 }
 
@@ skipping 219 matching lines @@
   // Dictionary has been allocated with sufficient size for all elements.
   DisallowHeapAllocation no_need_to_resize_dictionary;
   HandleScope scope(isolate());
   USE(UnseededNumberDictionary::AtNumberPut(
       dictionary_, IdToKey(ast_id), handle(target, isolate())));
 }
 
 
 }  // namespace internal
 }  // namespace v8