Improve inobject field tracking during GVN.

Now stores to inobject fields do no longer prevent hoisting
and combining of loads from other inobject fields.

R=mstarzinger@chromium.org

Review URL: https://codereview.chromium.org/144423010

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@19249 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/hydrogen-check-elimination.cc

@@ -103,8 +103,8 @@ class HCheckTable : public ZoneObject {
       }
       default: {
         // If the instruction changes maps uncontrollably, drop everything.
-        if (instr->CheckGVNFlag(kChangesMaps) ||
-            instr->CheckGVNFlag(kChangesOsrEntries)) {
+        if (instr->CheckChangesFlag(kMaps) ||
+            instr->CheckChangesFlag(kOsrEntries)) {
           Kill();
         }
       }
@@ -387,7 +387,7 @@ class HCheckTable : public ZoneObject {
       Insert(object, MapConstant(instr->value()));
     } else {
       // If the instruction changes maps, it should be handled above.
-      CHECK(!instr->CheckGVNFlag(kChangesMaps));
+      CHECK(!instr->CheckChangesFlag(kMaps));
     }
   }
 
@@ -571,8 +571,8 @@ class HCheckMapsEffects : public ZoneObject {
         maps_stored_ = true;
       }
       default: {
-        maps_stored_ |= (instr->CheckGVNFlag(kChangesMaps) |
-                         instr->CheckGVNFlag(kChangesElementsKind));
+        maps_stored_ |= (instr->CheckChangesFlag(kMaps) |
+                         instr->CheckChangesFlag(kElementsKind));
       }
     }
   }

src/hydrogen-gvn.h

@@ -36,15 +36,76 @@
 namespace v8 {
 namespace internal {
 
+// This class extends GVNFlagSet with additional "special" dynamic side effects,
+// which can be used to represent side effects that cannot be expressed using
+// the GVNFlags of an HInstruction. These special side effects are tracked by a
+// SideEffectsTracker (see below).
+class SideEffects V8_FINAL {
+ public:
+  static const int kNumberOfSpecials = 64 - kNumberOfFlags;
+
+  SideEffects() : bits_(0) {}
+  explicit SideEffects(GVNFlagSet flags) : bits_(flags.ToIntegral()) {}
+  bool IsEmpty() const { return bits_ == 0; }
+  bool ContainsFlag(GVNFlag flag) const {
+    return (bits_ & MaskFlag(flag)) != 0;
+  }
+  bool ContainsSpecial(int special) const {
+    return (bits_ & MaskSpecial(special)) != 0;
+  }
+  bool ContainsAnyOf(SideEffects set) const { return (bits_ & set.bits_) != 0; }
+  void Add(SideEffects set) { bits_ |= set.bits_; }
+  void AddSpecial(int special) { bits_ |= MaskSpecial(special); }
+  void AddAllSpecial() { bits_ |= ~static_cast<uint64_t>(0) << kNumberOfFlags; }
+  void RemoveFlag(GVNFlag flag) { bits_ &= ~MaskFlag(flag); }
+  void RemoveAll() { bits_ = 0; }
+  uint64_t ToIntegral() const { return bits_; }
+  void PrintTo(StringStream* stream) const;
+
+ private:
+  uint64_t MaskFlag(GVNFlag flag) const {
+    return static_cast<uint64_t>(1) << static_cast<unsigned>(flag);
+  }
+  uint64_t MaskSpecial(int special) const {
+    ASSERT(special >= 0);
+    ASSERT(special < kNumberOfSpecials);
+    return static_cast<uint64_t>(1) << static_cast<unsigned>(
+        special + kNumberOfFlags);
+  }
+
+  uint64_t bits_;
+  STATIC_ASSERT(kNumberOfFlags + kNumberOfSpecials == sizeof(bits_) * CHAR_BIT);
+};
+
+
+// Tracks inobject field loads/stores in a fine grained fashion, and represents
+// them using the "special" dynamic side effects of the SideEffects class (see
+// above). This way unrelated inobject field stores don't prevent hoisting and
+// merging of inobject field loads.
+class SideEffectsTracker V8_FINAL BASE_EMBEDDED {
+ public:
+  SideEffectsTracker() : num_inobject_fields_(0) {}
+  SideEffects ComputeChanges(HInstruction* instr);
+  SideEffects ComputeDependsOn(HInstruction* instr);
+  void PrintSideEffectsTo(StringStream* stream, SideEffects side_effects) const;
+
+ private:
+  bool ComputeInobjectField(HObjectAccess access, int* index);
+
+  HObjectAccess inobject_fields_[SideEffects::kNumberOfSpecials];
+  int num_inobject_fields_;
+};
+
+
 // Perform common subexpression elimination and loop-invariant code motion.
-class HGlobalValueNumberingPhase : public HPhase {
+class HGlobalValueNumberingPhase V8_FINAL : public HPhase {
  public:
   explicit HGlobalValueNumberingPhase(HGraph* graph);
 
   void Run();
 
  private:
-  GVNFlagSet CollectSideEffectsOnPathsToDominatedBlock(
+  SideEffects CollectSideEffectsOnPathsToDominatedBlock(
       HBasicBlock* dominator,
       HBasicBlock* dominated);
   void AnalyzeGraph();
@@ -52,17 +113,18 @@ class HGlobalValueNumberingPhase : public HPhase {
   void LoopInvariantCodeMotion();
   void ProcessLoopBlock(HBasicBlock* block,
                         HBasicBlock* before_loop,
-                        GVNFlagSet loop_kills);
+                        SideEffects loop_kills);
   bool AllowCodeMotion();
   bool ShouldMove(HInstruction* instr, HBasicBlock* loop_header);
 
+  SideEffectsTracker side_effects_tracker_;
   bool removed_side_effects_;
 
   // A map of block IDs to their side effects.
-  ZoneList<GVNFlagSet> block_side_effects_;
+  ZoneList<SideEffects> block_side_effects_;
 
   // A map of loop header block IDs to their loop's side effects.
-  ZoneList<GVNFlagSet> loop_side_effects_;
+  ZoneList<SideEffects> loop_side_effects_;
 
   // Used when collecting side effects on paths from dominator to
   // dominated.
@@ -71,7 +133,6 @@ class HGlobalValueNumberingPhase : public HPhase {
   DISALLOW_COPY_AND_ASSIGN(HGlobalValueNumberingPhase);
 };
 
-
 } }  // namespace v8::internal
 
 #endif  // V8_HYDROGEN_GVN_H_

src/hydrogen-instructions.cc

@@ -604,11 +604,11 @@ void HValue::PrintChangesTo(StringStream* stream) {
     stream->Add("*");
   } else {
     bool add_comma = false;
-#define PRINT_DO(type)                            \
-    if (changes_flags.Contains(kChanges##type)) { \
-      if (add_comma) stream->Add(",");            \
-      add_comma = true;                           \
-      stream->Add(#type);                         \
+#define PRINT_DO(Type)                      \
+    if (changes_flags.Contains(k##Type)) {  \
+      if (add_comma) stream->Add(",");      \
+      add_comma = true;                     \
+      stream->Add(#Type);                   \
     }
     GVN_TRACKED_FLAG_LIST(PRINT_DO);
     GVN_UNTRACKED_FLAG_LIST(PRINT_DO);
@@ -1516,7 +1516,7 @@ void HCheckInstanceType::GetCheckMaskAndTag(uint8_t* mask, uint8_t* tag) {
 
 bool HCheckMaps::HandleSideEffectDominator(GVNFlag side_effect,
                                            HValue* dominator) {
-  ASSERT(side_effect == kChangesMaps);
+  ASSERT(side_effect == kMaps);
   // TODO(mstarzinger): For now we specialize on HStoreNamedField, but once
   // type information is rich enough we should generalize this to any HType
   // for which the map is known.
@@ -1624,7 +1624,7 @@ Range* HChange::InferRange(Zone* zone) {
         input_range != NULL &&
         input_range->IsInSmiRange()))) {
     set_type(HType::Smi());
-    ClearGVNFlag(kChangesNewSpacePromotion);
+    ClearChangesFlag(kNewSpacePromotion);
   }
   Range* result = (input_range != NULL)
       ? input_range->Copy(zone)
@@ -3412,7 +3412,7 @@ Representation HUnaryMathOperation::RepresentationFromInputs() {
 
 bool HAllocate::HandleSideEffectDominator(GVNFlag side_effect,
                                           HValue* dominator) {
-  ASSERT(side_effect == kChangesNewSpacePromotion);
+  ASSERT(side_effect == kNewSpacePromotion);
   Zone* zone = block()->zone();
   if (!FLAG_use_allocation_folding) return false;
 
@@ -4394,52 +4394,76 @@ void HObjectAccess::SetGVNFlags(HValue *instr, PropertyAccessType access_type) {
   // set the appropriate GVN flags for a given load or store instruction
   if (access_type == STORE) {
     // track dominating allocations in order to eliminate write barriers
-    instr->SetGVNFlag(kDependsOnNewSpacePromotion);
+    instr->SetDependsOnFlag(::v8::internal::kNewSpacePromotion);
     instr->SetFlag(HValue::kTrackSideEffectDominators);
   } else {
     // try to GVN loads, but don't hoist above map changes
     instr->SetFlag(HValue::kUseGVN);
-    instr->SetGVNFlag(kDependsOnMaps);
+    instr->SetDependsOnFlag(::v8::internal::kMaps);
   }
 
   switch (portion()) {
     case kArrayLengths:
-      instr->SetGVNFlag(access_type == STORE
-          ? kChangesArrayLengths : kDependsOnArrayLengths);
+      if (access_type == STORE) {
+        instr->SetChangesFlag(::v8::internal::kArrayLengths);
+      } else {
+        instr->SetDependsOnFlag(::v8::internal::kArrayLengths);
+      }
       break;
     case kStringLengths:
-      instr->SetGVNFlag(access_type == STORE
-          ? kChangesStringLengths : kDependsOnStringLengths);
+      if (access_type == STORE) {
+        instr->SetChangesFlag(::v8::internal::kStringLengths);
+      } else {
+        instr->SetDependsOnFlag(::v8::internal::kStringLengths);
+      }
       break;
     case kInobject:
-      instr->SetGVNFlag(access_type == STORE
-          ? kChangesInobjectFields : kDependsOnInobjectFields);
+      if (access_type == STORE) {
+        instr->SetChangesFlag(::v8::internal::kInobjectFields);
+      } else {
+        instr->SetDependsOnFlag(::v8::internal::kInobjectFields);
+      }
       break;
     case kDouble:
-      instr->SetGVNFlag(access_type == STORE
-          ? kChangesDoubleFields : kDependsOnDoubleFields);
+      if (access_type == STORE) {
+        instr->SetChangesFlag(::v8::internal::kDoubleFields);
+      } else {
+        instr->SetDependsOnFlag(::v8::internal::kDoubleFields);
+      }
       break;
     case kBackingStore:
-      instr->SetGVNFlag(access_type == STORE
-          ? kChangesBackingStoreFields : kDependsOnBackingStoreFields);
+      if (access_type == STORE) {
+        instr->SetChangesFlag(::v8::internal::kBackingStoreFields);
+      } else {
+        instr->SetDependsOnFlag(::v8::internal::kBackingStoreFields);
+      }
       break;
     case kElementsPointer:
-      instr->SetGVNFlag(access_type == STORE
-          ? kChangesElementsPointer : kDependsOnElementsPointer);
+      if (access_type == STORE) {
+        instr->SetChangesFlag(::v8::internal::kElementsPointer);
+      } else {
+        instr->SetDependsOnFlag(::v8::internal::kElementsPointer);
+      }
       break;
     case kMaps:
-      instr->SetGVNFlag(access_type == STORE
-          ? kChangesMaps : kDependsOnMaps);
+      if (access_type == STORE) {
+        instr->SetChangesFlag(::v8::internal::kMaps);
+      } else {
+        instr->SetDependsOnFlag(::v8::internal::kMaps);
+      }
       break;
     case kExternalMemory:
-      instr->SetGVNFlag(access_type == STORE
-          ? kChangesExternalMemory : kDependsOnExternalMemory);
+      if (access_type == STORE) {
+        instr->SetChangesFlag(::v8::internal::kExternalMemory);
+      } else {
+        instr->SetDependsOnFlag(::v8::internal::kExternalMemory);
+      }
       break;
   }
 }
 
 
-void HObjectAccess::PrintTo(StringStream* stream) {
+void HObjectAccess::PrintTo(StringStream* stream) const {
   stream->Add(".");
 
   switch (portion()) {

src/hydrogen-load-elimination.cc

@@ -99,25 +99,25 @@ class HLoadEliminationTable : public ZoneObject {
         break;
       }
       default: {
-        if (instr->CheckGVNFlag(kChangesInobjectFields)) {
+        if (instr->CheckChangesFlag(kInobjectFields)) {
           TRACE((" kill-all i%d\n", instr->id()));
           Kill();
           break;
         }
-        if (instr->CheckGVNFlag(kChangesMaps)) {
+        if (instr->CheckChangesFlag(kMaps)) {
           TRACE((" kill-maps i%d\n", instr->id()));
           KillOffset(JSObject::kMapOffset);
         }
-        if (instr->CheckGVNFlag(kChangesElementsKind)) {
+        if (instr->CheckChangesFlag(kElementsKind)) {
           TRACE((" kill-elements-kind i%d\n", instr->id()));
           KillOffset(JSObject::kMapOffset);
           KillOffset(JSObject::kElementsOffset);
         }
-        if (instr->CheckGVNFlag(kChangesElementsPointer)) {
+        if (instr->CheckChangesFlag(kElementsPointer)) {
           TRACE((" kill-elements i%d\n", instr->id()));
           KillOffset(JSObject::kElementsOffset);
         }
-        if (instr->CheckGVNFlag(kChangesOsrEntries)) {
+        if (instr->CheckChangesFlag(kOsrEntries)) {
           TRACE((" kill-osr i%d\n", instr->id()));
           Kill();
         }
@@ -454,11 +454,11 @@ class HLoadEliminationEffects : public ZoneObject {
         elements_stored_ = true;
       }
       default: {
-        fields_stored_ |= instr->CheckGVNFlag(kChangesInobjectFields);
-        maps_stored_ |= instr->CheckGVNFlag(kChangesMaps);
-        maps_stored_ |= instr->CheckGVNFlag(kChangesElementsKind);
-        elements_stored_ |= instr->CheckGVNFlag(kChangesElementsKind);
-        elements_stored_ |= instr->CheckGVNFlag(kChangesElementsPointer);
+        fields_stored_ |= instr->CheckChangesFlag(kInobjectFields);
+        maps_stored_ |= instr->CheckChangesFlag(kMaps);
+        maps_stored_ |= instr->CheckChangesFlag(kElementsKind);
+        elements_stored_ |= instr->CheckChangesFlag(kElementsKind);
+        elements_stored_ |= instr->CheckChangesFlag(kElementsPointer);
       }
     }
   }

src/hydrogen.cc

@@ -2174,7 +2174,7 @@ HInstruction* HGraphBuilder::BuildUncheckedMonomorphicElementAccess(
   // generated store code.
   if ((elements_kind == FAST_HOLEY_ELEMENTS) ||
       (elements_kind == FAST_ELEMENTS && access_type == STORE)) {
-    checked_object->ClearGVNFlag(kDependsOnElementsKind);
+    checked_object->ClearDependsOnFlag(kElementsKind);
   }
 
   bool fast_smi_only_elements = IsFastSmiElementsKind(elements_kind);
@@ -2184,7 +2184,7 @@ HInstruction* HGraphBuilder::BuildUncheckedMonomorphicElementAccess(
       store_mode != STORE_NO_TRANSITION_HANDLE_COW) {
     HCheckMaps* check_cow_map = Add<HCheckMaps>(
         elements, isolate()->factory()->fixed_array_map(), top_info());
-    check_cow_map->ClearGVNFlag(kDependsOnElementsKind);
+    check_cow_map->ClearDependsOnFlag(kElementsKind);
   }
   HInstruction* length = NULL;
   if (is_js_array) {
@@ -2258,7 +2258,7 @@ HInstruction* HGraphBuilder::BuildUncheckedMonomorphicElementAccess(
       } else {
         HCheckMaps* check_cow_map = Add<HCheckMaps>(
             elements, isolate()->factory()->fixed_array_map(), top_info());
-        check_cow_map->ClearGVNFlag(kDependsOnElementsKind);
+        check_cow_map->ClearDependsOnFlag(kElementsKind);
       }
     }
   }
@@ -5309,7 +5309,7 @@ HInstruction* HOptimizedGraphBuilder::BuildStoreNamedField(
   if (transition_to_field) {
     HConstant* transition_constant = Add<HConstant>(info->transition());
     instr->SetTransition(transition_constant, top_info());
-    instr->SetGVNFlag(kChangesMaps);
+    instr->SetChangesFlag(kMaps);
   }
   return instr;
 }
@@ -6208,7 +6208,7 @@ HInstruction* HOptimizedGraphBuilder::BuildMonomorphicElementAccess(
   HCheckMaps* checked_object = Add<HCheckMaps>(object, map, top_info(),
                                                dependency);
   if (dependency) {
-    checked_object->ClearGVNFlag(kDependsOnElementsKind);
+    checked_object->ClearDependsOnFlag(kElementsKind);
   }
 
   if (access_type == STORE && map->prototype()->IsJSObject()) {
@@ -6688,7 +6688,7 @@ HInstruction* HGraphBuilder::BuildConstantMapCheck(Handle<JSObject> constant,
   AddInstruction(constant_value);
   HCheckMaps* check =
       Add<HCheckMaps>(constant_value, handle(constant->map()), info);
-  check->ClearGVNFlag(kDependsOnElementsKind);
+  check->ClearDependsOnFlag(kElementsKind);
   return check;
 }