Change the stackmap builder API.

Change the stackmap builder API.

It is inconvenient for the stackmap table builder to use a singleton bitmap
for all safepoints in a code object.  Instead, split the bitmap from the
table builder and allow a bitmap per safepoint.

Take this opportunity to rename StackmapBuilder => StackmapTableBuilder
because it builds the entire table of stackmaps for a code object.

R=vegorov@google.com
BUG=

Committed: https://code.google.com/p/dart/source/detail?r=10043

[Vyacheslav Egorov (Google), 2012-07-30 15:03:45]

lgtm

[siva, 2012-07-31 19:03:00]

DBC.

https://chromiumcodereview.appspot.com/10825077/diff/1/runtime/vm/code_descri...
File runtime/vm/code_descriptors.h (right):

https://chromiumcodereview.appspot.com/10825077/diff/1/runtime/vm/code_descri...
runtime/vm/code_descriptors.h:71: 
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.

https://chromiumcodereview.appspot.com/10825077/diff/1/runtime/vm/code_descri...
File runtime/vm/code_descriptors_test.cc (right):

https://chromiumcodereview.appspot.com/10825077/diff/1/runtime/vm/code_descri...
runtime/vm/code_descriptors_test.cc:74: EXPECT(stackmap->Get(2));
Case in point here, this code is not readable at all compared
to stackmap->SetSlotAsObject(0);

https://chromiumcodereview.appspot.com/10825077/diff/1/runtime/vm/code_descri...
runtime/vm/code_descriptors_test.cc:242: stackmap->Set(6, true);  // Second
argument to func(i, k).
Ditto comment about readability of this code compared to what it used to be.

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

https://chromiumcodereview.appspot.com/10825077/diff/1/runtime/vm/code_descri...
File runtime/vm/code_descriptors.h (right):

https://chromiumcodereview.appspot.com/10825077/diff/1/runtime/vm/code_descri...
runtime/vm/code_descriptors.h:71: 
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.