3887 - clean up early exits in interpreter loop

It's always confusing when `break` refers to a `switch` but `continue`
refers to the loop around the `switch`. But we've done ugly things like
this and `goto` for expedience. However, we're starting to run into cases
where we now need to insert code at every `continue` or `continue`-mimicking
`goto` inside the core interpreter loop. Better to make the loop single-entry-single-exit.
Common things to run after every instruction will now happen inside the
`finish_instruction` function rather than at the `finish_instruction` label.

7 files changed, 57 insertions, 33 deletions

diff --git a/020run.cc b/020run.cc
index 084cafbb..15644843 100644
--- a/020run.cc
+++ b/020run.cc
@@ -80,6 +80,12 @@ void run_current_routine() {
     }
     // instructions below will write to 'products'
     vector<vector<double> > products;
+    //: This will be a large switch that later layers will often insert cases
+    //: into. Never call 'continue' within it. Instead, we'll explicitly
+    //: control which of the following stages after the switch we run for each
+    //: instruction.
+    bool write_products = true;
+    bool fall_through_to_next_instruction = true;
     switch (current_instruction().operation) {
       // Primitive Recipe Implementations
       case COPY: {
@@ -92,18 +98,20 @@ void run_current_routine() {
       }
     }
     //: used by a later layer
-    Writing_products_of_instruction = true;
-    if (SIZE(products) < SIZE(current_instruction().products)) {
-      raise << SIZE(products) << " vs " << SIZE(current_instruction().products) << ": failed to write to all products in '" << to_original_string(current_instruction()) << "'\n" << end();
-    }
-    else {
-      for (int i = 0;  i < SIZE(current_instruction().products);  ++i)
-        write_memory(current_instruction().products.at(i), products.at(i));
+    if (write_products) {
+      Writing_products_of_instruction = true;
+      if (SIZE(products) < SIZE(current_instruction().products)) {
+        raise << SIZE(products) << " vs " << SIZE(current_instruction().products) << ": failed to write to all products in '" << to_original_string(current_instruction()) << "'\n" << end();
+      }
+      else {
+        for (int i = 0;  i < SIZE(current_instruction().products);  ++i)
+          write_memory(current_instruction().products.at(i), products.at(i));
+      }
+      Writing_products_of_instruction = false;
     }
-    Writing_products_of_instruction = false;
     // End Running One Instruction
-    finish_instruction:;
-    ++current_step_index();
+    if (fall_through_to_next_instruction)
+      ++current_step_index();
   }
   stop_running_current_routine:;
 }
diff --git a/024jump.cc b/024jump.cc
index 5e345eca..748b2405 100644
--- a/024jump.cc
+++ b/024jump.cc
@@ -30,7 +30,10 @@ case JUMP: {
   assert(current_instruction().ingredients.at(0).initialized);
   current_step_index() += ingredients.at(0).at(0)+1;
   trace(9998, "run") << "jumping to instruction " << current_step_index() << end();
-  continue;  // skip rest of this instruction
+  // skip rest of this instruction
+  write_products = false;
+  fall_through_to_next_instruction = false;
+  break;
 }
 
 //: special type to designate jump targets
@@ -78,7 +81,10 @@ case JUMP_IF: {
   }
   current_step_index() += ingredients.at(1).at(0)+1;
   trace(9998, "run") << "jumping to instruction " << current_step_index() << end();
-  continue;  // skip rest of this instruction
+  // skip rest of this instruction
+  write_products = false;
+  fall_through_to_next_instruction = false;
+  break;
 }
 
 :(scenario jump_if)
@@ -131,7 +137,10 @@ case JUMP_UNLESS: {
   }
   current_step_index() += ingredients.at(1).at(0)+1;
   trace(9998, "run") << "jumping to instruction " << current_step_index() << end();
-  continue;  // skip rest of this instruction
+  // skip rest of this instruction
+  write_products = false;
+  fall_through_to_next_instruction = false;
+  break;
 }
 
 :(scenario jump_unless)
diff --git a/026call.cc b/026call.cc
index dd9db69c..81012350 100644
--- a/026call.cc
+++ b/026call.cc
@@ -107,21 +107,20 @@ if (!contains_key(Recipe, inst.operation)) {
 }
 :(replace{} "default:" following "End Primitive Recipe Implementations")
 default: {
-  const instruction& call_instruction = current_instruction();
-  if (Recipe.find(current_instruction().operation) == Recipe.end()) {  // duplicate from Checks
-    // stop running this instruction immediately
-    ++current_step_index();
-    continue;
+  if (contains_key(Recipe, current_instruction().operation)) {  // error already raised in Checks above
+    // not a primitive; look up the book of recipes
+    if (Trace_stream) {
+      ++Trace_stream->callstack_depth;
+      trace(9999, "trace") << "incrementing callstack depth to " << Trace_stream->callstack_depth << end();
+      assert(Trace_stream->callstack_depth < 9000);  // 9998-101 plus cushion
+    }
+    const instruction& call_instruction = current_instruction();
+    Current_routine->calls.push_front(call(current_instruction().operation));
+    finish_call_housekeeping(call_instruction, ingredients);
+    // not done with caller
+    write_products = false;
+    fall_through_to_next_instruction = false;
   }
-  // not a primitive; look up the book of recipes
-  if (Trace_stream) {
-    ++Trace_stream->callstack_depth;
-    trace(9999, "trace") << "incrementing callstack depth to " << Trace_stream->callstack_depth << end();
-    assert(Trace_stream->callstack_depth < 9000);  // 9998-101 plus cushion
-  }
-  Current_routine->calls.push_front(call(current_instruction().operation));
-  finish_call_housekeeping(call_instruction, ingredients);
-  continue;  // not done with caller; don't increment step_index of caller
 }
 :(code)
 void finish_call_housekeeping(const instruction& call_instruction, const vector<vector<double> >& ingredients) {
diff --git a/030container.cc b/030container.cc
index 4cb147fe..b189435f 100644
--- a/030container.cc
+++ b/030container.cc
@@ -575,11 +575,12 @@ case PUT: {
   // optimization: directly write the element rather than updating 'product'
   // and writing the entire container
   // Write Memory in PUT in Run
+  write_products = false;
   for (int i = 0;  i < SIZE(ingredients.at(2));  ++i) {
     trace(9999, "mem") << "storing " << no_scientific(ingredients.at(2).at(i)) << " in location " << address+i << end();
     put(Memory, address+i, ingredients.at(2).at(i));
   }
-  goto finish_instruction;
+  break;
 }
 
 :(scenario put_product_error)
diff --git a/032array.cc b/032array.cc
index 778daa97..3b521263 100644
--- a/032array.cc
+++ b/032array.cc
@@ -66,7 +66,8 @@ case CREATE_ARRAY: {
     put(Memory, base_address+i, 0);
   }
   // no need to update product
-  goto finish_instruction;
+  write_products = false;
+  break;
 }
 
 :(scenario copy_array)
@@ -557,13 +558,14 @@ case PUT_INDEX: {
   trace(9998, "run") << "address to copy to is " << address << end();
   // optimization: directly write the element rather than updating 'product'
   // and writing the entire array
+  write_products = false;
   vector<double> value = read_memory(current_instruction().ingredients.at(2));
   // Write Memory in PUT_INDEX in Run
   for (int i = 0;  i < SIZE(value);  ++i) {
     trace(9999, "mem") << "storing " << no_scientific(value.at(i)) << " in location " << address+i << end();
     put(Memory, address+i, value.at(i));
   }
-  goto finish_instruction;
+  break;
 }
 
 :(scenario put_index_out_of_bounds)
diff --git a/033exclusive_container.cc b/033exclusive_container.cc
index eee16643..86dc1c8a 100644
--- a/033exclusive_container.cc
+++ b/033exclusive_container.cc
@@ -160,6 +160,7 @@ case MAYBE_CONVERT: {
   reagent/*copy*/ status = current_instruction().products.at(1);
   // Update MAYBE_CONVERT status in Run
   // optimization: directly write results to only update first product when necessary
+  write_products = false;
   if (tag == static_cast<int>(get_or_insert(Memory, base_address))) {
     const reagent& variant = variant_type(base, tag);
     trace(9999, "mem") << "storing 1 in location " << status.value << end();
@@ -177,7 +178,7 @@ case MAYBE_CONVERT: {
     trace(9999, "mem") << "storing 0 in location " << status.value << end();
     put(Memory, status.value, 0);
   }
-  goto finish_instruction;
+  break;
 }
 
 :(code)
diff --git a/071recipe.cc b/071recipe.cc
index 1a145ed6..c44d3127 100644
--- a/071recipe.cc
+++ b/071recipe.cc
@@ -114,13 +114,17 @@ case CALL: {
     raise << maybe(current_recipe_name()) << "tried to call empty recipe in '" << to_string(current_instruction()) << "'" << end();
     break;
   }
-  instruction/*copy*/ call_instruction = current_instruction();
+  const call& caller_frame = current_call();
+  instruction/*copy*/ call_instruction = to_instruction(caller_frame);
   call_instruction.operation = ingredients.at(0).at(0);
   call_instruction.ingredients.erase(call_instruction.ingredients.begin());
   Current_routine->calls.push_front(call(ingredients.at(0).at(0)));
   ingredients.erase(ingredients.begin());  // drop the callee
   finish_call_housekeeping(call_instruction, ingredients);
-  continue;
+  // not done with caller
+  write_products = false;
+  fall_through_to_next_instruction = false;
+  break;
 }
 
 //:: check types for 'call' instructions