Refactor Instruction classes to use a template base class.

vm/intermediate_language_ia32.cc

 // 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.
 
 #include "vm/globals.h"  // Needed here to get TARGET_ARCH_IA32.
 #if defined(TARGET_ARCH_IA32)
 
 #include "vm/intermediate_language.h"
 
 #include "lib/error.h"
@@ skipping 1005 matching lines @@
   compiler->GenerateInstanceOf(cid(),
                                token_pos(),
                                try_index(),
                                type(),
                                negate_result());
   ASSERT(locs()->out().reg() == EAX);
 }
 
 
 LocationSummary* CreateArrayComp::MakeLocationSummary() const {
-  return MakeCallSummary();
+  const intptr_t kNumInputs = 1;

[Kevin Millikin (Google), 2012/08/07 10:06:03]

I have a feeling that this is always InputCount() for all
instructions/computations.  I think it would be clearer to write:

new LocationSummary(InputCount(), kNumTemps, LocationSummary::kCall);

Let's consider that for a future change.

[Florian Schneider, 2012/08/07 10:23:55]

Yes, that will be the case once all call-instructions report the correct input
count via InputCount(). There are few instructions left, that don't right now,
but I'll fix those in my next couple of CLs.

+  const intptr_t kNumTemps = 0;
+  LocationSummary* locs =
+      new LocationSummary(kNumInputs, kNumTemps, LocationSummary::kCall);
+  locs->set_in(0, Location::RegisterLocation(ECX));

[Kevin Millikin (Google), 2012/08/07 10:06:03]

I don't really like the number '0' here.  It has to agree in a bunch of places
(the initializer in the constructor, the get accessor, here, and in the codegen
when it's used).  It might be easier to have a "layout" enum describing the
inputs:

enum Layout {
  kElementType,
};

...

locs->set_in(kElementType, Location::RegisterLocation(ECX));

========

Another option is to make Value into a pair of the SSA value and the location
and Definition contain the output location, rather than splitting
(non-temporary) locations out into a separate location summary.  I suppose we
still need a platform-specific list of indexed temporary locations, but
otherwise it seems cleaner.

In class Value:

Location& location() { return location_; }

In class CreateArrayComp:

Value* element_type() const { return inputs_[0]; }

...

// and here:

element_type().location() = Location::RegisterLocation(ECX);

[Florian Schneider, 2012/08/07 10:23:55]

Agreed. Let's consider removing those 0s and 1s in a future CL.

+  locs->set_out(Location::RegisterLocation(EAX));
+  return locs;
 }
 
 
 void CreateArrayComp::EmitNativeCode(FlowGraphCompiler* compiler) {
-  // TODO(fschneider): Support call-instructions that take inputs in registers.
   // Allocate the array.  EDX = length, ECX = element type.
-  __ popl(ECX);
+  ASSERT(locs()->in(0).reg() == ECX);

[Kevin Millikin (Google), 2012/08/07 10:06:03]

There's the 0 again.

   __ movl(EDX,  Immediate(Smi::RawValue(ElementCount())));
   compiler->GenerateCall(token_pos(),
                          try_index(),
                          &StubCode::AllocateArrayLabel(),
                          PcDescriptors::kOther);
   ASSERT(locs()->out().reg() == EAX);
 
   // Pop the element values from the stack into the array.
   __ leal(EDX, FieldAddress(EAX, Array::data_offset()));
   for (int i = ElementCount() - 1; i >= 0; --i) {
@@ skipping 1052 matching lines @@
   ASSERT(locs()->out().reg() == EAX);
   __ CompareObject(locs()->out().reg(), compiler->bool_true());
   EmitBranchOnCondition(compiler, branch_condition);
 }
 
 }  // namespace dart
 
 #undef __
 
 #endif  // defined TARGET_ARCH_X64