Use a uniform way to emit code for all instructions.

runtime/vm/intermediate_language.h

Index: runtime/vm/intermediate_language.h
diff --git a/runtime/vm/intermediate_language.h b/runtime/vm/intermediate_language.h
index 42c8470ad8ee9848ed160ea8c308a88b0cd9bc3b..af0ba4910843a92f2a2671b8379761f07637b9ff 100644
--- a/runtime/vm/intermediate_language.h
+++ b/runtime/vm/intermediate_language.h
@@ -637,7 +637,8 @@ class Instruction : public ZoneAllocated {
         previous_(NULL),
         next_(NULL),
         env_(NULL),
-        expr_id_(kNoExprId) { }
+        expr_id_(kNoExprId),
+        locs_(NULL) { }
 
   virtual Tag tag() const = 0;
 
@@ -739,11 +740,11 @@ FOR_EACH_INSTRUCTION(INSTRUCTION_TYPE_CHECK)
 
   // Returns structure describing location constraints required
   // to emit native code for this instruction.
-  virtual LocationSummary* locs() {
-    // TODO(vegorov): This should be pure virtual method.
-    // However we are temporary using NULL for instructions that
-    // were not converted to the location based code generation yet.
-    return NULL;
+  LocationSummary* locs() {
+    if (locs_ == NULL) {
+      locs_ = MakeLocationSummary();
+    }
+    return locs_;
   }
 
   virtual LocationSummary* MakeLocationSummary() const = 0;
@@ -927,6 +928,7 @@ FOR_EACH_INSTRUCTION(INSTRUCTION_TYPE_CHECK)
   Instruction* next_;
   Environment* env_;
   intptr_t expr_id_;
+  LocationSummary* locs_;

[Florian Schneider, 2013/05/22 15:56:15]

I'd prefer to have this member only on those that actually need a
LocationSummary.

[Kevin Millikin (Google), 2013/05/23 09:09:49]

Good: all instructions need a location summary --- to tell the compiler what
locations they use and whether they make a call.

I'd rather use a real location summary than have special case code to support
NULL as a redundant representation of new LocationSummary(0, 0,
LocationSummary::kNoCall).

[Florian Schneider, 2013/05/23 19:12:36]

Fine with me, if there is no visible impact on compilation time (e.g. when
running dart2js).

If now all instructions have a location summary, shouldn't then locs_ be just an
embedded LocationSummary member instead of a pointer to a ZoneAllocated object?

 
   DISALLOW_COPY_AND_ASSIGN(Instruction);
 };
@@ -935,18 +937,9 @@ FOR_EACH_INSTRUCTION(INSTRUCTION_TYPE_CHECK)
 template<intptr_t N>
 class TemplateInstruction: public Instruction {
  public:
-  TemplateInstruction<N>() : locs_(NULL) { }
-
   virtual intptr_t InputCount() const { return N; }
   virtual Value* InputAt(intptr_t i) const { return inputs_[i]; }
 
-  virtual LocationSummary* locs() {
-    if (locs_ == NULL) {
-      locs_ = MakeLocationSummary();
-    }
-    return locs_;
-  }
-
  protected:
   EmbeddedArray<Value*, N> inputs_;
 
@@ -954,8 +947,6 @@ class TemplateInstruction: public Instruction {
   virtual void RawSetInputAt(intptr_t i, Value* value) {
     inputs_[i] = value;
   }
-
-  LocationSummary* locs_;
 };
 
 
@@ -1147,12 +1138,6 @@ class BlockEntryInstr : public Instruction {
 
   virtual intptr_t ArgumentCount() const { return 0; }
 
-  virtual bool CanBeDeoptimizationTarget() const {
-    // BlockEntry environment is copied to Goto and Branch instructions
-    // when we insert new blocks targeting this block.
-    return true;
-  }
-
   virtual bool CanDeoptimize() const { return false; }
 
   virtual EffectSet Effects() const { return EffectSet::None(); }
@@ -1368,6 +1353,12 @@ class JoinEntryInstr : public BlockEntryInstr {
   virtual EffectSet Effects() const { return EffectSet::None(); }
   virtual EffectSet Dependencies() const { return EffectSet::None(); }
 
+  virtual bool CanBeDeoptimizationTarget() const {
+    // The JoinEntry's environment is copied to Goto and Branch instructions
+    // when we insert new blocks targeting this block.
+    return true;
+  }
+
  private:
   // Classes that have access to predecessors_ when inlining.
   friend class BlockEntryInstr;
@@ -1428,6 +1419,12 @@ class TargetEntryInstr : public BlockEntryInstr {
 
   virtual void PrintTo(BufferFormatter* f) const;
 
+  virtual bool CanBeDeoptimizationTarget() const {
+    // The TargetEntry's environment is copied to Goto and Branch
+    // instructions when we insert new blocks targeting this block.
+    return true;
+  }
+
  private:
   friend class BlockEntryInstr;  // Access to predecessor_ when inlining.
 
@@ -1784,7 +1781,7 @@ class ParameterInstr : public Definition {
 
 class PushArgumentInstr : public Definition {

[Florian Schneider, 2013/05/22 15:56:15]

Can't PushArgument just inherit from TemplateDefinition<1>?

  public:
-  explicit PushArgumentInstr(Value* value) : locs_(NULL) {
+  explicit PushArgumentInstr(Value* value) {
     SetInputAt(0, value);
     set_use_kind(kEffect);  // Override the default.
   }
@@ -1803,13 +1800,6 @@ class PushArgumentInstr : public Definition {
 
   Value* value() const { return value_; }
 
-  virtual LocationSummary* locs() {
-    if (locs_ == NULL) {
-      locs_ = MakeLocationSummary();
-    }
-    return locs_;
-  }
-
   virtual intptr_t Hashcode() const {
     UNREACHABLE();
     return 0;
@@ -1830,7 +1820,6 @@ class PushArgumentInstr : public Definition {
   }
 
   Value* value_;
-  LocationSummary* locs_;
 
   DISALLOW_COPY_AND_ASSIGN(PushArgumentInstr);
 };
@@ -2020,7 +2009,6 @@ class BranchInstr : public ControlInstruction {
 
   bool is_checked() const { return is_checked_; }
 
-  virtual LocationSummary* locs();
   virtual intptr_t DeoptimizationTarget() const;
   virtual Representation RequiredInputRepresentation(intptr_t i) const;
 
@@ -2100,20 +2088,11 @@ class StoreContextInstr : public TemplateInstruction<1> {
 template<intptr_t N>
 class TemplateDefinition : public Definition {
  public:
-  TemplateDefinition<N>() : locs_(NULL) { }
+  TemplateDefinition<N>() { }
 
   virtual intptr_t InputCount() const { return N; }
   virtual Value* InputAt(intptr_t i) const { return inputs_[i]; }
 
-  // Returns a structure describing the location constraints required
-  // to emit native code for this definition.
-  LocationSummary* locs() {
-    if (locs_ == NULL) {
-      locs_ = MakeLocationSummary();
-    }
-    return locs_;
-  }
-
  protected:
   EmbeddedArray<Value*, N> inputs_;
 
@@ -2123,8 +2102,6 @@ class TemplateDefinition : public Definition {
   virtual void RawSetInputAt(intptr_t i, Value* value) {
     inputs_[i] = value;
   }
-
-  LocationSummary* locs_;
 };
 
 
@@ -2763,17 +2740,6 @@ inline EffectSet BranchInstr::Effects() const {
 }
 
 
-inline LocationSummary* BranchInstr::locs() {
-  if (comparison()->locs_ == NULL) {
-    LocationSummary* summary = comparison()->MakeLocationSummary();
-    // Branches don't produce a result.
-    summary->set_out(Location::NoLocation());
-    comparison()->locs_ = summary;
-  }
-  return comparison()->locs_;
-}
-
-
 inline intptr_t BranchInstr::DeoptimizationTarget() const {
   return comparison()->DeoptimizationTarget();
 }
@@ -3689,11 +3655,6 @@ class MaterializeObjectInstr : public Definition {
   virtual bool CanDeoptimize() const { return false; }
   virtual EffectSet Effects() const { return EffectSet::None(); }
 
-  LocationSummary* locs() {
-    UNREACHABLE();
-    return NULL;
-  }
-
   Location* locations() { return locations_; }
   void set_locations(Location* locations) { locations_ = locations; }
 
@@ -6034,15 +5995,6 @@ class InvokeMathCFunctionInstr : public Definition {
     return (*inputs_)[i];
   }
 
-  // Returns a structure describing the location constraints required
-  // to emit native code for this definition.
-  LocationSummary* locs() {
-    if (locs_ == NULL) {
-      locs_ = MakeLocationSummary();
-    }
-    return locs_;
-  }
-
   virtual bool AllowsCSE() const { return true; }
   virtual EffectSet Effects() const { return EffectSet::None(); }
   virtual EffectSet Dependencies() const { return EffectSet::None(); }
@@ -6060,8 +6012,6 @@ class InvokeMathCFunctionInstr : public Definition {
 
   ZoneGrowableArray<Value*>* inputs_;
 
-  LocationSummary* locs_;
-
   const MethodRecognizer::Kind recognized_kind_;
 
   DISALLOW_COPY_AND_ASSIGN(InvokeMathCFunctionInstr);