Improve inobject field tracking during GVN.

src/hydrogen-gvn.h

Index: src/hydrogen-gvn.h
diff --git a/src/hydrogen-gvn.h b/src/hydrogen-gvn.h
index 30333cca61045d7292496cece8aa3235c173e7fb..47d96f7c5922cc5aab46e86139f5a2cbdc0dd5bb 100644
--- a/src/hydrogen-gvn.h
+++ b/src/hydrogen-gvn.h
@@ -36,15 +36,68 @@
 namespace v8 {
 namespace internal {
 
+class SideEffects V8_FINAL {

[Michael Starzinger, 2014/02/10 14:50:07]

A comment that this is merely a GVNFlagSet plus "special" dynamic side effects
and why we need that would be nice.

[Benedikt Meurer, 2014/02/11 06:40:54]

Done.

+ 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);
+};
+
+
+class SideEffectsScope V8_FINAL BASE_EMBEDDED {

[Michael Starzinger, 2014/02/10 14:50:07]

Can we have a few comments above this class to explain what it is used for? Also
the name is misleading IMHO, as it sounds like several of these scopes are
stacked (which they aren't). It is only used to track finer-grained side-effects
for loads and stores. How about calling it SideEffectsMap (my favorite),
SideEffectsCache or SideEffectsTracker instead?

[Benedikt Meurer, 2014/02/11 06:40:54]

Done.

Went for SideEffectsTracker, because we already have a HSideEffectsMap.

+ public:
+  SideEffectsScope() : 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 +105,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);
 
+  SideEffectsScope side_effects_scope_;
   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 +125,6 @@ class HGlobalValueNumberingPhase : public HPhase {
   DISALLOW_COPY_AND_ASSIGN(HGlobalValueNumberingPhase);
 };
 
-
 } }  // namespace v8::internal
 
 #endif  // V8_HYDROGEN_GVN_H_