Change the stackmap builder API.

runtime/vm/code_descriptors.h

 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #ifndef VM_CODE_DESCRIPTORS_H_
 #define VM_CODE_DESCRIPTORS_H_
 
 #include "vm/globals.h"
 #include "vm/growable_array.h"
 #include "vm/object.h"
@@ skipping 41 matching lines @@
                      intptr_t try_index);
 
   RawPcDescriptors* FinalizePcDescriptors(uword entry_point);
 
  private:
   GrowableArray<struct PcDesc> list_;
   DISALLOW_COPY_AND_ASSIGN(DescriptorList);
 };
 
 
-class StackmapBuilder : public ZoneAllocated {
+class StackmapTableBuilder : public ZoneAllocated {
  public:
-  StackmapBuilder() :
-      builder_(new BitmapBuilder()),
+  StackmapTableBuilder() :
       stack_map_(Stackmap::ZoneHandle()),
       list_(GrowableObjectArray::ZoneHandle(
           GrowableObjectArray::New(Heap::kOld))) { }
-  ~StackmapBuilder() { }
+  ~StackmapTableBuilder() { }
 
-  // Gets state of stack slot (object or regular value).
-  bool IsSlotObject(intptr_t stack_slot) const {
-    ASSERT(builder_ != NULL);
-    return builder_->Get(stack_slot);
-  }
-  // Sets stack slot as containing an object.
-  void SetSlotAsObject(intptr_t stack_slot) {
-    ASSERT(builder_ != NULL);
-    builder_->Set(stack_slot, true);
-  }
-  // Sets stack slot as containing regular value.
-  void SetSlotAsValue(intptr_t stack_slot) {
-    ASSERT(builder_ != NULL);
-    builder_->Set(stack_slot, false);
-  }
-  // Sets min..max (inclusive) as stack slots containing objects.
-  void SetSlotRangeAsObject(intptr_t min_stack_slot, intptr_t max_stack_slot) {
-    ASSERT(builder_ != NULL);
-    builder_->SetRange(min_stack_slot, max_stack_slot, true);
-  }
-  // Sets min..max (inclusive) as stack slots containing regular values.
-  void SetSlotRangeAsValue(intptr_t min_stack_slot, intptr_t max_stack_slot) {
-    ASSERT(builder_ != NULL);
-    builder_->SetRange(min_stack_slot, max_stack_slot, false);
-  }
-
-  void AddEntry(intptr_t pc_offset);
+  void AddEntry(intptr_t pc_offset, BitmapBuilder* bitmap);
 

[siva, 2012/07/31 19:03:01]

I would retain a StackmapBuilder class which has a BitmapBuilder field in it and
defines all the methods that you have deleted here:
IsSlotObject,
SetSlotAsObject methods.
...
...
The AddEntry method would take a StackmapBuilder object and not a BitmapBuilder
object.

This would make the stackmap generation code much more readable than the current
form of just setting true, false etc. in the bitmap.

[Kevin Millikin (Google), 2012/08/01 09:34:33]

Thanks for the comment.

I agree with the naming issue (Slava raised it to), but a separate wrapper class
seems too heavy.  There will be exactly one call in the code to SetSlotAsObject,
one to IsSlotObject, and none at all to the other methods:

for (intptr_t i = 0; i < safepoints_.length(); ++i) {
  if (range->CanCover(safepoints_[i].position)) {
    safepoints_[i].stack_bitmap->Set(spill_index, true);
  }
}

(Replace Set(spill_index, true) by SetSlotAsObject(spill_index)).  It seems too
much to wrap the whole bitmap builder, when the interpretation is pretty obvious
in the code above.

I'll hold off on making any change for now, but a lightweight solution for
better naming might be a static function on the StackmapTableBuilder.

   bool Verify();
 
   RawArray* FinalizeStackmaps(const Code& code);
 
  private:
   intptr_t Length() const { return list_.Length(); }
   RawStackmap* Map(int index) const;
 
-  BitmapBuilder* builder_;
   Stackmap& stack_map_;
   GrowableObjectArray& list_;
-  DISALLOW_COPY_AND_ASSIGN(StackmapBuilder);
+  DISALLOW_COPY_AND_ASSIGN(StackmapTableBuilder);
 };
 
 
 class ExceptionHandlerList : public ZoneAllocated {
  public:
   struct HandlerDesc {
     intptr_t try_index;  // Try block index handled by the handler.
     intptr_t pc_offset;  // Handler PC offset value.
   };
 
@@ skipping 31 matching lines @@
   }
 
  private:
   GrowableArray<struct HandlerDesc> list_;
   DISALLOW_COPY_AND_ASSIGN(ExceptionHandlerList);
 };
 
 }  // namespace dart
 
 #endif  // VM_CODE_DESCRIPTORS_H_