Use a uniform way to emit code for all instructions.

runtime/vm/intermediate_language.h

 // Copyright (c) 2013, 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_INTERMEDIATE_LANGUAGE_H_
 #define VM_INTERMEDIATE_LANGUAGE_H_
 
 #include "vm/allocation.h"
 #include "vm/ast.h"
 #include "vm/growable_array.h"
@@ skipping 619 matching lines @@
     FOR_EACH_INSTRUCTION(DECLARE_TAG)
   };
 #undef DECLARE_TAG
 
   Instruction()
       : deopt_id_(Isolate::Current()->GetNextDeoptId()),
         lifetime_position_(-1),
         previous_(NULL),
         next_(NULL),
         env_(NULL),
-        expr_id_(kNoExprId) { }
+        expr_id_(kNoExprId),
+        locs_(NULL) { }
 
   virtual Tag tag() const = 0;
 
   intptr_t deopt_id() const {
     ASSERT(CanDeoptimize() || CanBeDeoptimizationTarget());
     return deopt_id_;
   }
 
   ICData* GetICData(const Array& ic_data_array) const;
 
@@ skipping 81 matching lines @@
   virtual void PrintOperandsTo(BufferFormatter* f) const;
 
 #define INSTRUCTION_TYPE_CHECK(type)                                           \
   bool Is##type() { return (As##type() != NULL); }                             \
   virtual type##Instr* As##type() { return NULL; }
 FOR_EACH_INSTRUCTION(INSTRUCTION_TYPE_CHECK)
 #undef 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;
 
   static LocationSummary* MakeCallSummary();
 
   virtual void EmitNativeCode(FlowGraphCompiler* compiler) {
     UNIMPLEMENTED();
   }
 
@@ skipping 163 matching lines @@
   enum {
     kNoExprId = -1
   };
 
   intptr_t deopt_id_;
   intptr_t lifetime_position_;  // Position used by register allocator.
   Instruction* previous_;
   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);
 };
 
 
 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_;
 
  private:
   virtual void RawSetInputAt(intptr_t i, Value* value) {
     inputs_[i] = value;
   }
-
-  LocationSummary* locs_;
 };
 
 
 class MoveOperands : public ZoneAllocated {
  public:
   MoveOperands(Location dest, Location src) : dest_(dest), src_(src) { }
 
   Location src() const { return src_; }
   Location dest() const { return dest_; }
 
@@ skipping 171 matching lines @@
       intptr_t fixed_parameter_count);
 
   virtual intptr_t InputCount() const { return 0; }
   virtual Value* InputAt(intptr_t i) const {
     UNREACHABLE();
     return NULL;
   }
 
   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(); }
   virtual EffectSet Dependencies() const { return EffectSet::None(); }
 
   virtual bool MayThrow() const { return false; }
 
   intptr_t try_index() const { return try_index_; }
 
   // True for blocks inside a try { } region.
@@ skipping 195 matching lines @@
   void InsertPhi(intptr_t var_index, intptr_t var_count);
   void RemoveDeadPhis(Definition* replacement);
 
   void InsertPhi(PhiInstr* phi);
 
   virtual void PrintTo(BufferFormatter* f) const;
 
   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;
   friend class InlineExitCollector;
   friend class PolymorphicInliner;
 
   // Direct access to phis_ in order to resize it due to phi elimination.
   friend class ConstantPropagator;
 
   virtual void ClearPredecessors() { predecessors_.Clear(); }
@@ skipping 40 matching lines @@
   virtual intptr_t PredecessorCount() const {
     return (predecessor_ == NULL) ? 0 : 1;
   }
   virtual BlockEntryInstr* PredecessorAt(intptr_t index) const {
     ASSERT((index == 0) && (predecessor_ != NULL));
     return predecessor_;
   }
 
   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.
 
   virtual void ClearPredecessors() { predecessor_ = NULL; }
   virtual void AddPredecessor(BlockEntryInstr* predecessor) {
     ASSERT(predecessor_ == NULL);
     predecessor_ = predecessor;
   }
 
   BlockEntryInstr* predecessor_;
@@ skipping 334 matching lines @@
  private:
   virtual void RawSetInputAt(intptr_t i, Value* value) { UNREACHABLE(); }
 
   const intptr_t index_;
   BlockEntryInstr* block_;
 
   DISALLOW_COPY_AND_ASSIGN(ParameterInstr);
 };
 
 
 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.
   }
 
   DECLARE_INSTRUCTION(PushArgument)
 
   intptr_t InputCount() const { return 1; }
   Value* InputAt(intptr_t i) const {
     ASSERT(i == 0);
     return value_;
   }
 
   virtual intptr_t ArgumentCount() const { return 0; }
 
   virtual CompileType ComputeType() const;
 
   Value* value() const { return value_; }
 
-  virtual LocationSummary* locs() {
-    if (locs_ == NULL) {
-      locs_ = MakeLocationSummary();
-    }
-    return locs_;
-  }
-
   virtual intptr_t Hashcode() const {
     UNREACHABLE();
     return 0;
   }
 
   virtual bool CanDeoptimize() const { return false; }
 
   virtual EffectSet Effects() const { return EffectSet::None(); }
 
   virtual void PrintOperandsTo(BufferFormatter* f) const;
 
   virtual bool MayThrow() const { return false; }
 
  private:
   virtual void RawSetInputAt(intptr_t i, Value* value) {
     ASSERT(i == 0);
     value_ = value;
   }
 
   Value* value_;
-  LocationSummary* locs_;
 
   DISALLOW_COPY_AND_ASSIGN(PushArgumentInstr);
 };
 
 
 inline Definition* Instruction::ArgumentAt(intptr_t index) const {
   return PushArgumentAt(index)->value()->definition();
 }
 
 
@@ skipping 169 matching lines @@
   virtual bool CanDeoptimize() const;
   virtual bool CanBeDeoptimizationTarget() const;
 
   virtual EffectSet Effects() const;
 
   ComparisonInstr* comparison() const { return comparison_; }
   void SetComparison(ComparisonInstr* comp);
 
   bool is_checked() const { return is_checked_; }
 
-  virtual LocationSummary* locs();
   virtual intptr_t DeoptimizationTarget() const;
   virtual Representation RequiredInputRepresentation(intptr_t i) const;
 
   // A misleadingly named function for use in template functions that also
   // replace definitions.  In this case, leave the branch intact and replace
   // its comparison with another comparison that has been removed from the
   // graph but still has uses properly linked into their definition's use
   // list.
   void ReplaceWith(ComparisonInstr* other,
                    ForwardInstructionIterator* ignored);
@@ skipping 59 matching lines @@
   virtual bool MayThrow() const { return false; }
 
  private:
   DISALLOW_COPY_AND_ASSIGN(StoreContextInstr);
 };
 
 
 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_;
 
  private:
   friend class BranchInstr;
 
   virtual void RawSetInputAt(intptr_t i, Value* value) {
     inputs_[i] = value;
   }
-
-  LocationSummary* locs_;
 };
 
 
 class RedefinitionInstr : public TemplateDefinition<1> {
  public:
   explicit RedefinitionInstr(Value* value) {
     SetInputAt(0, value);
   }
 
   DECLARE_INSTRUCTION(Redefinition)
@@ skipping 618 matching lines @@
 inline bool BranchInstr::CanBeDeoptimizationTarget() const {
   return comparison()->CanBeDeoptimizationTarget();
 }
 
 
 inline EffectSet BranchInstr::Effects() const {
   return comparison()->Effects();
 }
 
 
-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();
 }
 
 
 inline Representation BranchInstr::RequiredInputRepresentation(
     intptr_t i) const {
   return comparison()->RequiredInputRepresentation(i);
 }
 
@@ skipping 895 matching lines @@
     return values_->length();
   }
 
   virtual Value* InputAt(intptr_t i) const {
     return (*values_)[i];
   }
 
   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; }
 
   virtual bool MayThrow() const { return false; }
 
  private:
   virtual void RawSetInputAt(intptr_t i, Value* value) {
     (*values_)[i] = value;
   }
 
@@ skipping 2320 matching lines @@
   virtual intptr_t DeoptimizationTarget() const { return deopt_id_; }
 
   virtual intptr_t InputCount() const {
     return inputs_->length();
   }
 
   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_;
-  }
-
   virtual bool AllowsCSE() const { return true; }
   virtual EffectSet Effects() const { return EffectSet::None(); }
   virtual EffectSet Dependencies() const { return EffectSet::None(); }
   virtual bool AttributesEqual(Instruction* other) const {
     InvokeMathCFunctionInstr* other_invoke = other->AsInvokeMathCFunction();
     return other_invoke->recognized_kind() == recognized_kind();
   }
 
   virtual bool MayThrow() const { return false; }
 
  private:
   virtual void RawSetInputAt(intptr_t i, Value* value) {
     (*inputs_)[i] = value;
   }
 
   ZoneGrowableArray<Value*>* inputs_;
 
-  LocationSummary* locs_;
-
   const MethodRecognizer::Kind recognized_kind_;
 
   DISALLOW_COPY_AND_ASSIGN(InvokeMathCFunctionInstr);
 };
 
 
 class CheckClassInstr : public TemplateInstruction<1> {
  public:
   CheckClassInstr(Value* value,
                   intptr_t deopt_id,
@@ skipping 331 matching lines @@
   ForwardInstructionIterator* current_iterator_;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(FlowGraphVisitor);
 };
 
 
 }  // namespace dart
 
 #endif  // VM_INTERMEDIATE_LANGUAGE_H_