From 7a29cf544e73fbd516b2e6118c093fd5df81a274 Mon Sep 17 00:00:00 2001 From: "Kartik K. Agaram" Date: Tue, 16 Aug 2016 14:30:19 -0700 Subject: 3197 Replace an integer with a boolean across two layers of function calls. It has long been one of the ugliest consequences of my approach with layers that functions might need to be introduced with unnecessary arguments simply because we have no clean way to add parameters to a function definition after the fact -- or to add the default argument corresponding to that parameter in calls. This problem is exacerbated by the redundant argument having to be passed in through multiple layers of functions. In this instance: In layer 20 we define write_memory with an argument called 'saving_instruction_products' which isn't used yet. In layer 36 we reveal that we use this argument in a call to should_update_refcounts_in_write_memory() -- where it is again not used yet. Layer 43 finally clarifies what we're shooting for: a) In general when we need to update some memory, we always want to update refcounts. b) The only exception is when we're reclaiming locals in a function that set up its stack frame using 'local-scope' (signalling that it wants immediate reclamation). At that point we avoid decrementing refcounts of 'escaping' addresses that are being returned, and we also avoid incrementing refcounts of products in the caller instruction. The latter case is basically why we need this boolean and its dance across 3 layers. In summary, write_memory() needs to update refcounts except if: we're writing products for an instruction, the instruction is not a primitive, and the (callee) recipe for the instruction starts with 'local-scope'. --- 020run.cc | 4 ++-- 036refcount.cc | 4 ++-- 043space.cc | 9 ++++----- 074deep_copy.cc | 2 +- 4 files changed, 9 insertions(+), 10 deletions(-) diff --git a/020run.cc b/020run.cc index 143cc07c..77a132a9 100644 --- a/020run.cc +++ b/020run.cc @@ -91,7 +91,7 @@ void run_current_routine() } else { for (int i = 0; i < SIZE(current_instruction().products); ++i) - write_memory(current_instruction().products.at(i), products.at(i), i); + write_memory(current_instruction().products.at(i), products.at(i), /*saving instruction products rather than some other internal uses*/true); } // End of Instruction finish_instruction:; @@ -277,7 +277,7 @@ vector read_memory(reagent/*copy*/ x) { return result; } -void write_memory(reagent/*copy*/ x, const vector& data, const int /*only when called in the run loop above to save results; -1 otherwise*/ product_index) { +void write_memory(reagent/*copy*/ x, const vector& data, const bool saving_instruction_products) { // Begin Preprocess write_memory(x, data) if (!x.type) { raise << "can't write to '" << to_string(x) << "'; no type\n" << end(); diff --git a/036refcount.cc b/036refcount.cc index 638aed24..51a0945a 100644 --- a/036refcount.cc +++ b/036refcount.cc @@ -18,7 +18,7 @@ def main [ +mem: decrementing refcount of 1000: 1 -> 0 :(before "End write_memory(x) Special-cases") -if (should_update_refcounts_in_write_memory(product_index)) { +if (should_update_refcounts_in_write_memory(saving_instruction_products)) { if (is_mu_address(x)) { assert(scalar(data)); assert(x.value); @@ -30,7 +30,7 @@ if (should_update_refcounts_in_write_memory(product_index)) { :(code) //: hook for a later layer -bool should_update_refcounts_in_write_memory(int product_index) { +bool should_update_refcounts_in_write_memory(bool conditional_update) { return true; } diff --git a/043space.cc b/043space.cc index 275640cd..bb221201 100644 --- a/043space.cc +++ b/043space.cc @@ -247,7 +247,7 @@ void try_reclaim_locals() { if (escaping(inst.products.at(i))) continue; trace(9999, "mem") << "clearing " << inst.products.at(i).original_string << end(); zeros.resize(size_of(inst.products.at(i))); - write_memory(inst.products.at(i), zeros, /*always update refcounts*/-1); + write_memory(inst.products.at(i), zeros, /*always update refcounts*/false); } } trace(9999, "mem") << "automatically abandoning " << current_call().default_space << end(); @@ -259,11 +259,10 @@ void try_reclaim_locals() { //: since we don't decrement refcounts for escaping values above, make sure we //: don't increment them when the caller saves them either -:(replace{} "bool should_update_refcounts_in_write_memory(int product_index)") -bool should_update_refcounts_in_write_memory(int product_index) { +:(replace{} "bool should_update_refcounts_in_write_memory(bool conditional_update)") +bool should_update_refcounts_in_write_memory(bool conditional_update) { assert(Current_routine); // run-time only - if (product_index == -1) return true; - assert(product_index >= 0); + if (!conditional_update) return true; const instruction& inst = current_instruction(); if (inst.operation < MAX_PRIMITIVE_RECIPES) return true; if (!contains_key(Recipe, inst.operation)) return true; diff --git a/074deep_copy.cc b/074deep_copy.cc index 59f71286..34100de5 100644 --- a/074deep_copy.cc +++ b/074deep_copy.cc @@ -264,7 +264,7 @@ int deep_copy_address(const reagent& canonized_in, map& addresses_copi payload.value = tmp.value; // now modified for output vector old_data = read_memory(payload); trace(9991, "run") << "deep-copy: really writing to " << payload.value << ' ' << to_string(payload) << " (old value " << to_string(old_data) << " new value " << to_string(data) << ")" << end(); - write_memory(payload, data, -1); + write_memory(payload, data, /*always update refcounts*/false); trace(9991, "run") << "deep-copy: output is " << to_string(data) << end(); return out; } -- cgit 1.4.1-2-gfad0