Improve inobject field tracking during GVN.

src/hydrogen-gvn.h

 // Copyright 2013 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 18 matching lines @@
 #define V8_HYDROGEN_GVN_H_
 
 #include "hydrogen.h"
 #include "hydrogen-instructions.h"
 #include "compiler.h"
 #include "zone.h"
 
 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();
   void ComputeBlockSideEffects();
   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.
   BitVector visited_on_paths_;
 
   DISALLOW_COPY_AND_ASSIGN(HGlobalValueNumberingPhase);
 };
 
-
 } }  // namespace v8::internal
 
 #endif  // V8_HYDROGEN_GVN_H_