Skip to content

Commit

Permalink
Bugfix: ASSIGNED variables are not considered for Localizer
Browse files Browse the repository at this point in the history
- 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
  • Loading branch information
pramodk committed Oct 16, 2023
1 parent 2355a6e commit 481efbe
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 2 deletions.
5 changes: 4 additions & 1 deletion src/visitors/defuse_analyze_visitor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
8 changes: 7 additions & 1 deletion src/visitors/localize_visitor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,8 @@ std::vector<std::string> 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
Expand Down Expand Up @@ -112,13 +113,18 @@ void LocalizeVisitor::visit_program(const ast::Program& node) {
const auto& blocks = node.get_blocks();
std::map<DUState, std::vector<std::shared_ptr<ast::Node>>> 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)) {
DefUseAnalyzeVisitor v(*program_symtab, ignore_verbatim);
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());
}
}

Expand Down
30 changes: 30 additions & 0 deletions test/unit/visitor/defuse_analyze.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 481efbe

Please sign in to comment.