From 481efbe30cfed4b1cc335def7402422a5fb769aa Mon Sep 17 00:00:00 2001 From: Pramod S Kumbhar Date: Tue, 17 Oct 2023 00:09:35 +0200 Subject: [PATCH] Bugfix: ASSIGNED variables are not considered for Localizer - previously only RANGE and GLOBAL variables were considered for localiser pass - exclude STATE variables as they are not usually a good candidate for such optimisations --- src/visitors/defuse_analyze_visitor.cpp | 5 ++++- src/visitors/localize_visitor.cpp | 8 ++++++- test/unit/visitor/defuse_analyze.cpp | 30 +++++++++++++++++++++++++ 3 files changed, 41 insertions(+), 2 deletions(-) diff --git a/src/visitors/defuse_analyze_visitor.cpp b/src/visitors/defuse_analyze_visitor.cpp index 7388d090de..859e981e55 100644 --- a/src/visitors/defuse_analyze_visitor.cpp +++ b/src/visitors/defuse_analyze_visitor.cpp @@ -422,10 +422,13 @@ DUChain DefUseAnalyzeVisitor::analyze(const ast::Ast& node, const std::string& n visiting_lhs = false; current_symtab = global_symtab; unsupported_node = false; + + // variable types that we try to localise: RANGE, GLOBAL and ASSIGNED + auto global_symbol_properties = NmodlType::global_var | NmodlType::range_var | + NmodlType::assigned_definition; auto global_symbol = global_symtab->lookup_in_scope(variable_name); // If global_symbol exists in the global_symtab then search for a global variable. Otherwise the // variable can only be local if it exists - auto global_symbol_properties = NmodlType::global_var | NmodlType::range_var; if (global_symbol != nullptr && global_symbol->has_any_property(global_symbol_properties)) { variable_type = DUVariableType::Global; } else { diff --git a/src/visitors/localize_visitor.cpp b/src/visitors/localize_visitor.cpp index 0181940b57..a9f71d71d7 100644 --- a/src/visitors/localize_visitor.cpp +++ b/src/visitors/localize_visitor.cpp @@ -77,7 +77,8 @@ std::vector LocalizeVisitor::variables_to_optimize() const { | NmodlType::nonspecific_cur_var | NmodlType::pointer_var | NmodlType::bbcore_pointer_var - | NmodlType::electrode_cur_var; + | NmodlType::electrode_cur_var + | NmodlType::state_var; const NmodlType global_var_properties = NmodlType::range_var | NmodlType::assigned_definition @@ -112,6 +113,8 @@ void LocalizeVisitor::visit_program(const ast::Program& node) { const auto& blocks = node.get_blocks(); std::map>> block_usage; + logger->debug("LocalizeVisitor: Checking DU chains for {}", varname); + /// compute def use chains for (const auto& block: blocks) { if (node_for_def_use_analysis(*block)) { @@ -119,6 +122,9 @@ void LocalizeVisitor::visit_program(const ast::Program& node) { const auto& usages = v.analyze(*block, varname); auto result = usages.eval(); block_usage[result].push_back(block); + logger->debug("\tDU chain in block {} is {}", + block->get_node_type_name(), + usages.to_string()); } } diff --git a/test/unit/visitor/defuse_analyze.cpp b/test/unit/visitor/defuse_analyze.cpp index cf0a36d0eb..6c20ca9bf6 100644 --- a/test/unit/visitor/defuse_analyze.cpp +++ b/test/unit/visitor/defuse_analyze.cpp @@ -445,6 +445,36 @@ SCENARIO("Perform DefUse analysis on NMODL constructs") { } } + GIVEN("Simple check of assigned variables") { + const std::string nmodl_text = R"( + NEURON { + GLOBAL x + } + + ASSIGNED { + y + } + + DERIVATIVE states { + x = 1 + y = x + y = y + 2 + } + )"; + + const std::string expected_text_y = + R"({"DerivativeBlock":[{"name":"D"},{"name":"U"},{"name":"D"}]})"; + + THEN("assigned variables are correctly analyzed") { + const std::string input = reindent_text(nmodl_text); + // Assigned variable "y" should be DU as it's defined and used as well + const auto& chains_y = run_defuse_visitor(input, "y"); + REQUIRE(chains_y[0].to_string() == expected_text_y); + REQUIRE(chains_y[0].eval() == DUState::D); + } + } + + GIVEN("global variable definition in if-else block") { std::string nmodl_text = R"( NEURON {