[vm] Refactor hash maps with T* keys where T <: Object.

Rename PointerKeyValueTrait<T>, which is used to create sets of pointers
to T instances where T <: Object and T has appropriate Hash and Equals
instance methods, to the more specific name PointerSetKeyValueTrait.

Create a PointerSet<T> alias for using this trait with
DirectChainedHashMap and use that alias in other code as a shorthand.

Remove PointerSetKeyValueTrait<const char> as a superclass of
CStringKeyValueTrait, as the only reuse from the former are the two
methods KeyOf and ValueOf which just return their argument, and having
this relationship is odd since const char is not a subtype of Object.

TEST=Renaming/refactoring, so existing tests.

Change-Id: I0274b16cb9fcb3939a28fb109fb8626c1ac8c0e9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/215761
Commit-Queue: Tess Strickland <[email protected]>
Reviewed-by: Martin Kustermann <[email protected]>

Author: sstrickl

runtime/vm/compiler/backend/range_analysis.cc

@@ -642,8 +642,6 @@ class Scheduler {
   }
 
  private:
-  typedef DirectChainedHashMap<PointerKeyValueTrait<Instruction> > Map;
-
   Instruction* EmitRecursively(Instruction* instruction, Instruction* sink) {
     // Schedule all unscheduled inputs and unwrap all constrained inputs.
     for (intptr_t i = 0; i < instruction->InputCount(); i++) {
@@ -723,7 +721,7 @@ class Scheduler {
   }
 
   FlowGraph* flow_graph_;
-  Map map_;
+  PointerSet<Instruction> map_;
   const ZoneGrowableArray<BlockEntryInstr*>& loop_headers_;
   GrowableArray<BlockEntryInstr*> pre_headers_;
   GrowableArray<Instruction*> emitted_;

runtime/vm/compiler/backend/redundancy_elimination.cc

@@ -46,9 +46,7 @@ class CSEInstructionMap : public ValueObject {
   }
 
  private:
-  typedef DirectChainedHashMap<PointerKeyValueTrait<Instruction> > Map;
-
-  Map map_;
+  PointerSet<Instruction> map_;
 };
 
 // Place describes an abstract location (e.g. field) that IR can load
@@ -683,7 +681,7 @@ class PhiPlaceMoves : public ZoneAllocated {
 class AliasedSet : public ZoneAllocated {
  public:
   AliasedSet(Zone* zone,
-             DirectChainedHashMap<PointerKeyValueTrait<Place> >* places_map,
+             PointerSet<Place>* places_map,
              ZoneGrowableArray<Place*>* places,
              PhiPlaceMoves* phi_moves)
       : zone_(zone),
@@ -1179,7 +1177,7 @@ class AliasedSet : public ZoneAllocated {
 
   Zone* zone_;
 
-  DirectChainedHashMap<PointerKeyValueTrait<Place> >* places_map_;
+  PointerSet<Place>* places_map_;
 
   const ZoneGrowableArray<Place*>& places_;
 
@@ -1188,7 +1186,7 @@ class AliasedSet : public ZoneAllocated {
   // A list of all seen aliases and a map that allows looking up canonical
   // alias object.
   GrowableArray<const Place*> aliases_;
-  DirectChainedHashMap<PointerKeyValueTrait<const Place> > aliases_map_;
+  PointerSet<const Place> aliases_map_;
 
   SmallSet<Place::ElementSize> typed_data_access_sizes_;
 
@@ -1244,9 +1242,8 @@ static bool IsPhiDependentPlace(Place* place) {
 // corresponding to phi input are numbered and record outgoing phi moves
 // for each block which establish correspondence between phi dependent place
 // and phi input's place that is flowing in.
-static PhiPlaceMoves* ComputePhiMoves(
-    DirectChainedHashMap<PointerKeyValueTrait<Place> >* map,
-    ZoneGrowableArray<Place*>* places) {
+static PhiPlaceMoves* ComputePhiMoves(PointerSet<Place>* map,
+                                      ZoneGrowableArray<Place*>* places) {
   Thread* thread = Thread::Current();
   Zone* zone = thread->zone();
   PhiPlaceMoves* phi_moves = new (zone) PhiPlaceMoves();
@@ -1300,10 +1297,9 @@ DART_FORCE_INLINE static intptr_t GetPlaceId(const Instruction* instr) {
 
 enum CSEMode { kOptimizeLoads, kOptimizeStores };
 
-static AliasedSet* NumberPlaces(
-    FlowGraph* graph,
-    DirectChainedHashMap<PointerKeyValueTrait<Place> >* map,
-    CSEMode mode) {
+static AliasedSet* NumberPlaces(FlowGraph* graph,
+                                PointerSet<Place>* map,
+                                CSEMode mode) {
   // Loads representing different expression ids will be collected and
   // used to build per offset kill sets.
   Zone* zone = graph->zone();
@@ -1735,7 +1731,7 @@ class LoadOptimizer : public ValueObject {
       return false;
     }
 
-    DirectChainedHashMap<PointerKeyValueTrait<Place> > map;
+    PointerSet<Place> map;
     AliasedSet* aliased_set = NumberPlaces(graph, &map, kOptimizeLoads);
     if ((aliased_set != NULL) && !aliased_set->IsEmpty()) {
       // If any loads were forwarded return true from Optimize to run load
@@ -2774,7 +2770,7 @@ class LoadOptimizer : public ValueObject {
   }
 
   FlowGraph* graph_;
-  DirectChainedHashMap<PointerKeyValueTrait<Place> >* map_;
+  PointerSet<Place>* map_;
 
   // Mapping between field offsets in words and expression ids of loads from
   // that offset.
@@ -2866,7 +2862,7 @@ class StoreOptimizer : public LivenessAnalysis {
  public:
   StoreOptimizer(FlowGraph* graph,
                  AliasedSet* aliased_set,
-                 DirectChainedHashMap<PointerKeyValueTrait<Place> >* map)
+                 PointerSet<Place>* map)
       : LivenessAnalysis(aliased_set->max_place_id(), graph->postorder()),
         graph_(graph),
         map_(map),
@@ -2887,7 +2883,7 @@ class StoreOptimizer : public LivenessAnalysis {
       return;
     }
 
-    DirectChainedHashMap<PointerKeyValueTrait<Place> > map;
+    PointerSet<Place> map;
     AliasedSet* aliased_set = NumberPlaces(graph, &map, kOptimizeStores);
     if ((aliased_set != NULL) && !aliased_set->IsEmpty()) {
       StoreOptimizer store_optimizer(graph, aliased_set, &map);
@@ -3048,7 +3044,7 @@ class StoreOptimizer : public LivenessAnalysis {
   }
 
   FlowGraph* graph_;
-  DirectChainedHashMap<PointerKeyValueTrait<Place> >* map_;
+  PointerSet<Place>* map_;
 
   // Mapping between field offsets in words and expression ids of loads from
   // that offset.

runtime/vm/compiler/backend/slot.cc

@@ -43,7 +43,7 @@ class SlotCache : public ZoneAllocated {
       : zone_(thread->zone()), fields_(thread->zone()) {}
 
   Zone* const zone_;
-  DirectChainedHashMap<PointerKeyValueTrait<const Slot> > fields_;
+  PointerSet<const Slot> fields_;
 };
 
 #define NATIVE_SLOT_NAME(C, F) Kind::k##C##_##F

runtime/vm/hash_map.h

@@ -359,21 +359,21 @@ class ZoneDirectChainedHashMap
 };
 
 template <typename T>
-class PointerKeyValueTrait {
+class PointerSetKeyValueTrait {
  public:
   typedef T* Value;
   typedef T* Key;
   typedef T* Pair;
 
   static Key KeyOf(Pair kv) { return kv; }
-
   static Value ValueOf(Pair kv) { return kv; }
-
   static inline uword Hash(Key key) { return key->Hash(); }
-
   static inline bool IsKeyEqual(Pair kv, Key key) { return kv->Equals(*key); }
 };
 
+template <typename T>
+using PointerSet = DirectChainedHashMap<PointerSetKeyValueTrait<T>>;
+
 template <typename T>
 class NumbersKeyValueTrait {
  public:
@@ -408,12 +408,14 @@ class RawPointerKeyValueTrait {
   static bool IsKeyEqual(Pair kv, Key key) { return kv.key == key; }
 };
 
-class CStringSetKeyValueTrait : public PointerKeyValueTrait<const char> {
+class CStringSetKeyValueTrait {
  public:
-  using Key = PointerKeyValueTrait<const char>::Key;
-  using Value = PointerKeyValueTrait<const char>::Value;
-  using Pair = PointerKeyValueTrait<const char>::Pair;
+  using Key = const char*;
+  using Value = const char*;
+  using Pair = const char*;
 
+  static Key KeyOf(Pair kv) { return kv; }
+  static Value ValueOf(Pair kv) { return kv; }
   static uword Hash(Key key) {
     ASSERT(key != nullptr);
     return Utils::StringHash(key, strlen(key));

runtime/vm/hash_map_test.cc

@@ -19,7 +19,7 @@ class TestValue {
 };
 
 TEST_CASE(DirectChainedHashMap) {
-  DirectChainedHashMap<PointerKeyValueTrait<TestValue> > map;
+  DirectChainedHashMap<PointerSetKeyValueTrait<TestValue>> map;
   EXPECT(map.IsEmpty());
   TestValue v1(0);
   TestValue v2(1);
@@ -33,14 +33,14 @@ TEST_CASE(DirectChainedHashMap) {
   EXPECT(map.Remove(&v1));
   EXPECT(map.Lookup(&v1) == NULL);
   map.Insert(&v1);
-  DirectChainedHashMap<PointerKeyValueTrait<TestValue> > map2(map);
+  DirectChainedHashMap<PointerSetKeyValueTrait<TestValue>> map2(map);
   EXPECT(map2.LookupValue(&v1) == &v1);
   EXPECT(map2.LookupValue(&v2) == &v2);
   EXPECT(map2.LookupValue(&v3) == &v1);
 }
 
 TEST_CASE(DirectChainedHashMapInsertRemove) {
-  DirectChainedHashMap<PointerKeyValueTrait<TestValue> > map;
+  DirectChainedHashMap<PointerSetKeyValueTrait<TestValue>> map;
   EXPECT(map.IsEmpty());
   TestValue v1(1);
   TestValue v2(3);  // Note: v1, v2, v3 should have the same hash.
@@ -97,7 +97,7 @@ TEST_CASE(DirectChainedHashMapInsertRemove) {
 }
 
 TEST_CASE(MallocDirectChainedHashMap) {
-  MallocDirectChainedHashMap<PointerKeyValueTrait<TestValue> > map;
+  MallocDirectChainedHashMap<PointerSetKeyValueTrait<TestValue>> map;
   EXPECT(map.IsEmpty());
   TestValue v1(0);
   TestValue v2(1);
@@ -108,7 +108,7 @@ TEST_CASE(MallocDirectChainedHashMap) {
   EXPECT(map.LookupValue(&v1) == &v1);
   EXPECT(map.LookupValue(&v2) == &v2);
   EXPECT(map.LookupValue(&v3) == &v1);
-  MallocDirectChainedHashMap<PointerKeyValueTrait<TestValue> > map2(map);
+  MallocDirectChainedHashMap<PointerSetKeyValueTrait<TestValue>> map2(map);
   EXPECT(map2.LookupValue(&v1) == &v1);
   EXPECT(map2.LookupValue(&v2) == &v2);
   EXPECT(map2.LookupValue(&v3) == &v1);
@@ -117,7 +117,7 @@ TEST_CASE(MallocDirectChainedHashMap) {
 TEST_CASE(ZoneDirectChainedHashMap) {
   auto zone = thread->zone();
   auto const map = new (zone)
-      ZoneDirectChainedHashMap<PointerKeyValueTrait<TestValue>>(zone);
+      ZoneDirectChainedHashMap<PointerSetKeyValueTrait<TestValue>>(zone);
   EXPECT(map->IsEmpty());
   TestValue v1(0);
   TestValue v2(1);

runtime/vm/program_visitor.cc

@@ -646,7 +646,7 @@ void ProgramVisitor::NormalizeAndDedupCompressedStackMaps(Thread* thread) {
   class NormalizeAndDedupCompressedStackMapsVisitor
       : public CodeVisitor,
         public Dedupper<CompressedStackMaps,
-                        PointerKeyValueTrait<const CompressedStackMaps>> {
+                        PointerSetKeyValueTrait<const CompressedStackMaps>> {
    public:
     NormalizeAndDedupCompressedStackMapsVisitor(Zone* zone,
                                                 IsolateGroup* isolate_group)