VM: Explicitly load function and context before calling a closure.

runtime/vm/flow_graph_builder.cc

Index: runtime/vm/flow_graph_builder.cc
===================================================================
--- runtime/vm/flow_graph_builder.cc	(revision 35546)
+++ runtime/vm/flow_graph_builder.cc	(working copy)
@@ -2376,39 +2376,62 @@
 }
 
 
-ClosureCallInstr* EffectGraphVisitor::BuildClosureCall(
-    ClosureCallNode* node) {
+void EffectGraphVisitor::BuildClosureCall(
+    ClosureCallNode* node, bool result_needed) {
   ValueGraphVisitor for_closure(owner());
   node->closure()->Visit(&for_closure);
   Append(for_closure);
-  PushArgumentInstr* push_closure = PushArgument(for_closure.value());
 
+  LocalVariable* tmp_var = EnterTempLocalScope(for_closure.value());
+
   ZoneGrowableArray<PushArgumentInstr*>* arguments =
       new ZoneGrowableArray<PushArgumentInstr*>(node->arguments()->length());
+  Value* closure_val = Bind(new LoadLocalInstr(*tmp_var));
+  PushArgumentInstr* push_closure = PushArgument(closure_val);
   arguments->Add(push_closure);
   BuildPushArguments(*node->arguments(), arguments);
 
   // Save context around the call.
   ASSERT(owner()->parsed_function()->saved_current_context_var() != NULL);
   BuildSaveContext(*owner()->parsed_function()->saved_current_context_var());
-  return new ClosureCallInstr(node, arguments);
+  closure_val = Bind(new LoadLocalInstr(*tmp_var));
+  Value* context_val = Bind(new LoadFieldInstr(closure_val,
+                                               Closure::context_offset(),
+                                               AbstractType::ZoneHandle(),
+                                               true));  // Immutable.
+  AddInstruction(new StoreContextInstr(context_val));
+  closure_val = Bind(new LoadLocalInstr(*tmp_var));
+  Value* function_val = Bind(new LoadFieldInstr(closure_val,
+                                                Closure::function_offset(),
+                                                AbstractType::ZoneHandle(),
+                                                true));  // Immutable.
+  Definition* closure_call =
+      new ClosureCallInstr(function_val, node, arguments);
+  if (result_needed) {
+    Value* result = Bind(closure_call);
+    Do(new StoreLocalInstr(*tmp_var, result));
+    // Restore context from temp.
+    BuildRestoreContext(
+        *owner()->parsed_function()->saved_current_context_var());
+    ReturnDefinition(ExitTempLocalScope(tmp_var));
+

[srdjan, 2014/04/30 15:41:58]

Why the empty line?

[Florian Schneider, 2014/04/30 16:02:19]

Done.

+  } else {
+    Do(closure_call);
+    // Restore context from saved location.
+    BuildRestoreContext(
+        *owner()->parsed_function()->saved_current_context_var());
+    Do(ExitTempLocalScope(tmp_var));
+  }
 }
 
 
 void EffectGraphVisitor::VisitClosureCallNode(ClosureCallNode* node) {
-  Do(BuildClosureCall(node));
-  // Restore context from saved location.
-  ASSERT(owner()->parsed_function()->saved_current_context_var() != NULL);
-  BuildRestoreContext(*owner()->parsed_function()->saved_current_context_var());
+  BuildClosureCall(node, false);
 }
 
 
 void ValueGraphVisitor::VisitClosureCallNode(ClosureCallNode* node) {
-  Value* result = Bind(BuildClosureCall(node));
-  // Restore context from temp.
-  ASSERT(owner()->parsed_function()->saved_current_context_var() != NULL);
-  BuildRestoreContext(*owner()->parsed_function()->saved_current_context_var());
-  ReturnValue(result);
+  BuildClosureCall(node, true);
 }