Change the HGraphBuilder to dispatch on the context.

src/hydrogen.h

Index: src/hydrogen.h
diff --git a/src/hydrogen.h b/src/hydrogen.h
index 91f3c9e2d632d8b20eee26627aadb35c1749b6a8..5611d8da182505fe64167816466e847bf4b5094c 100644
--- a/src/hydrogen.h
+++ b/src/hydrogen.h
@@ -557,10 +557,29 @@ class AstContext {
   bool IsValue() const { return kind_ == Expression::kValue; }
   bool IsTest() const { return kind_ == Expression::kTest; }
 
+  // 'Fill' this context with a hydrogen value.  The value is assumed to
+  // have already been inserted in the instruction stream (or not need to
+  // be, e.g., HPhi).  Call this function in tail position in the Visit
+  // functions for expressions.

[Kasper Lund, 2010/12/09 11:40:08]

I guess you can only 'fill' a context once. Would it make sense to verify that
this is the way it is used by having a bool is_filled_ in debug mode?

[Kevin Millikin (Chromium), 2010/12/09 12:45:54]

I'm not sure about that---it depends on how much state contexts hold.  For
instance, we might decide that the context surrounding 'e0 ? e1 : e2' gets
filled twice.

I'll hold off on asserting anything.

+  virtual void ReturnValue(HValue* value) = 0;
+
+  // Add a hydrogen instruction to the instruction stream (recording an
+  // environment simulation if necessary) and then fill this context with
+  // the instruction as value.
+  virtual void ReturnInstruction(HInstruction* instr, int ast_id) = 0;
+
  protected:
   AstContext(HGraphBuilder* owner, Expression::Context kind);
   virtual ~AstContext();
 
+  HGraphBuilder* owner() const { return owner_; }
+
+  // We want to be able to assert, in a context-specific way, that the stack
+  // height makes sense when the context is filled.
+#ifdef DEBUG
+  int original_count_;

[Kasper Lund, 2010/12/09 11:40:08]

The comment mentions stack height. The name mentions count. Maybe somehow link
the two?

+#endif
+
  private:
   HGraphBuilder* owner_;
   Expression::Context kind_;
@@ -573,6 +592,10 @@ class EffectContext: public AstContext {
   explicit EffectContext(HGraphBuilder* owner)
       : AstContext(owner, Expression::kEffect) {
   }
+  virtual ~EffectContext();
+
+  virtual void ReturnValue(HValue* value);
+  virtual void ReturnInstruction(HInstruction* instr, int ast_id);
 };
 
 
@@ -581,6 +604,10 @@ class ValueContext: public AstContext {
   explicit ValueContext(HGraphBuilder* owner)
       : AstContext(owner, Expression::kValue) {
   }
+  virtual ~ValueContext();
+
+  virtual void ReturnValue(HValue* value);
+  virtual void ReturnInstruction(HInstruction* instr, int ast_id);
 };
 
 
@@ -598,6 +625,9 @@ class TestContext: public AstContext {
         invert_false_(invert_false) {
   }
 
+  virtual void ReturnValue(HValue* value);
+  virtual void ReturnInstruction(HInstruction* instr, int ast_id);
+
   static TestContext* cast(AstContext* context) {
     ASSERT(context->IsTest());
     return reinterpret_cast<TestContext*>(context);
@@ -610,6 +640,10 @@ class TestContext: public AstContext {
   bool invert_false() { return invert_false_; }
 
  private:
+  // Build the shared core part of the translation unpacking a value into
+  // control flow.
+  void BuildBranch(HValue* value);
+
   HBasicBlock* if_true_;
   HBasicBlock* if_false_;
   bool invert_true_;
@@ -631,9 +665,25 @@ class HGraphBuilder: public AstVisitor {
 
   HGraph* CreateGraph(CompilationInfo* info);
 
+  // Simple accessors.
+  HGraph* graph() const { return graph_; }
+  HSubgraph* subgraph() const { return current_subgraph_; }
+
+  HEnvironment* environment() const { return subgraph()->environment(); }

[Kasper Lund, 2010/12/09 11:40:08]

Long term maybe this should be Environment?

[Kevin Millikin (Chromium), 2010/12/09 12:45:54]

Probably.  Or else we should get rid of the two or three layers of inlined
helpers, track the current running environment in the graph builder (there's no
great reason for it to be in the subgraph) and write directly:

env()->Push(...)

or whatever.

+  HBasicBlock* CurrentBlock() const { return subgraph()->exit_block(); }
+
+  // Adding instructions.
+  HInstruction* AddInstruction(HInstruction* instr);
+  void AddSimulate(int id);
+
+  // Bailout environment manipulation.
+  void Push(HValue* value) { environment()->Push(value); }
+  HValue* Pop() { return environment()->Pop(); }
+
  private:
   // Type of a member function that generates inline code for a native function.
-  typedef void (HGraphBuilder::*InlineFunctionGenerator)(int argument_count);
+  typedef void (HGraphBuilder::*InlineFunctionGenerator)(int argument_count,
+                                                         int ast_id);
 
   // Forward declarations for inner scope classes.
   class SubgraphScope;
@@ -650,19 +700,14 @@ class HGraphBuilder: public AstVisitor {
 
   // Simple accessors.
   TypeFeedbackOracle* oracle() const { return oracle_; }
-  HGraph* graph() const { return graph_; }
-  HSubgraph* subgraph() const { return current_subgraph_; }
   AstContext* ast_context() const { return ast_context_; }
   void set_ast_context(AstContext* context) { ast_context_ = context; }
   AstContext* call_context() const { return call_context_; }
   HBasicBlock* function_return() const { return function_return_; }
-  HEnvironment* environment() const { return subgraph()->environment(); }
-
-  HBasicBlock* CurrentBlock() const { return subgraph()->exit_block(); }
 
   // Generators for inline runtime functions.
-#define INLINE_FUNCTION_GENERATOR_DECLARATION(Name, argc, ressize)          \
-    void Generate##Name(int argument_count);
+#define INLINE_FUNCTION_GENERATOR_DECLARATION(Name, argc, ressize)      \
+  void Generate##Name(int argument_count, int ast_id);
 
   INLINE_FUNCTION_LIST(INLINE_FUNCTION_GENERATOR_DECLARATION)
   INLINE_RUNTIME_FUNCTION_LIST(INLINE_FUNCTION_GENERATOR_DECLARATION)
@@ -683,8 +728,6 @@ class HGraphBuilder: public AstVisitor {
                               HSubgraph* true_graph,
                               HSubgraph* false_graph);
 
-  void Push(HValue* value) { environment()->Push(value); }
-  HValue* Pop() { return environment()->Pop(); }
   HValue* Top() const { return environment()->Top(); }
   void Drop(int n) { environment()->Drop(n); }
   void Bind(Variable* var, HValue* value) { environment()->Bind(var, value); }
@@ -708,18 +751,15 @@ class HGraphBuilder: public AstVisitor {
   HValue* VisitArgument(Expression* expr);
   void VisitArgumentList(ZoneList<Expression*>* arguments);
 
-  HInstruction* AddInstruction(HInstruction* instr);
-  void AddSimulate(int id);
   void AddPhi(HPhi* phi);
 
   void PushAndAdd(HInstruction* instr);
-  void PushAndAdd(HInstruction* instr, int position);
 
   void PushArgumentsForStubCall(int argument_count);
 
-  // Initialize the arguments to the call based on then environment, add it
-  // to the graph, and drop the arguments from the environment.
-  void ProcessCall(HCall* call, int source_position);
+  // Remove the arguments from the bailout environment and emit instructions
+  // to push them as outgoing parameters.
+  void ProcessCall(HCall* call);
 
   void AssumeRepresentation(HValue* value, Representation r);
   static Representation ToRepresentation(TypeInfo info);
@@ -743,7 +783,7 @@ class HGraphBuilder: public AstVisitor {
                                    FunctionLiteral* function);
 
   // Helpers for flow graph construction.
-  void LookupGlobalPropertyCell(VariableProxy* expr,
+  void LookupGlobalPropertyCell(Variable* var,
                                 LookupResult* lookup,
                                 bool is_store);
 
@@ -753,10 +793,11 @@ class HGraphBuilder: public AstVisitor {
   bool TryMathFunctionInline(Call* expr);
   void TraceInline(Handle<JSFunction> target, bool result);
 
-  void HandleGlobalVariableAssignment(VariableProxy* proxy,
+  void HandleGlobalVariableAssignment(Variable* var,
                                       HValue* value,
-                                      int position);
-  void HandleGlobalVariableLoad(VariableProxy* expr);
+                                      int position,
+                                      int ast_id);
+
   void HandlePropertyAssignment(Assignment* expr);
   void HandleCompoundAssignment(Assignment* expr);
   void HandlePolymorphicLoadNamedField(Property* expr,