Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set function member in instructions #1843

Merged
merged 2 commits into from
Feb 15, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/analyses/dependence_graph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ void dep_graph_domaint::control_dependencies(
from->is_assume())
control_deps.insert(from);

const irep_idt id=goto_programt::get_function_id(from);
const irep_idt id=from->function;
const cfg_post_dominatorst &pd=dep_graph.cfg_post_dominators().at(id);

// check all candidates for M
Expand Down
4 changes: 2 additions & 2 deletions src/goto-instrument/full_slicer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ void full_slicert::add_jumps(
continue;
}

const irep_idt id=goto_programt::get_function_id(j.PC);
const irep_idt id=j.PC->function;
const cfg_post_dominatorst &pd=post_dominators.at(id);

cfg_post_dominatorst::cfgt::entry_mapt::const_iterator e=
Expand Down Expand Up @@ -184,7 +184,7 @@ void full_slicert::add_jumps(

if(cfg[entry->second].node_required)
{
const irep_idt id2=goto_programt::get_function_id(*d_it);
const irep_idt id2=(*d_it)->function;
INVARIANT(id==id2,
"goto/jump expected to be within a single function");

Expand Down
5 changes: 2 additions & 3 deletions src/goto-programs/goto_convert_functions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -213,9 +213,8 @@ void goto_convert_functionst::convert_function(
// add "end of function"
f.body.destructive_append(tmp_end_function);

// do function tags
Forall_goto_program_instructions(i_it, f.body)
i_it->function=identifier;
// do function tags (they are empty at this point)
f.update_instructions_function(identifier);

f.body.update();

Expand Down
19 changes: 19 additions & 0 deletions src/goto-programs/goto_functions_template.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,14 @@ class goto_function_templatet
parameter_identifiers.clear();
}

/// update the function member in each instruction
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you elaborate this comment to explain how the instructions_function are being updated.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added param description

/// \param function_id: the `function_id` used for assigning empty function
/// members
void update_instructions_function(const irep_idt &function_id)
{
body.update_instructions_function(function_id);
}

void swap(goto_function_templatet &other)
{
body.swap(other.body);
Expand Down Expand Up @@ -149,12 +157,23 @@ class goto_functions_templatet
void compute_target_numbers();
void compute_incoming_edges();

/// update the function member in each instruction by setting it to
/// the goto function's identifier
void update_instructions_function()
{
for(auto &func : function_map)
{
func.second.update_instructions_function(func.first);
}
}

void update()
{
compute_incoming_edges();
compute_target_numbers();
compute_location_numbers();
compute_loop_numbers();
update_instructions_function();
}

static inline irep_idt entry_point()
Expand Down
7 changes: 7 additions & 0 deletions src/goto-programs/goto_model.h
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,13 @@ class goto_model_functiont
goto_functions.compute_location_numbers(goto_function.body);
}

/// Updates the empty function member of each instruction by setting them
/// to `function_id`
void update_instructions_function()
{
goto_function.update_instructions_function(function_id);
}

/// Get symbol table
/// \return journalling symbol table that (a) wraps the global symbol table,
/// and (b) has recorded all symbol mutations (insertions, updates and
Expand Down
29 changes: 18 additions & 11 deletions src/goto-programs/goto_program_template.h
Original file line number Diff line number Diff line change
Expand Up @@ -386,21 +386,12 @@ class goto_program_templatet
return t;
}

static const irep_idt get_function_id(
const_targett l)
{
while(!l->is_end_function())
++l;

return l->function;
}

static const irep_idt get_function_id(
const goto_program_templatet<codeT, guardT> &p)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this use the typedef'd name?

{
assert(!p.empty());
PRECONDITION(!p.empty());

return get_function_id(--p.instructions.end());
return p.instructions.back().function;
Copy link
Member Author

@peterschrammel peterschrammel Feb 15, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tautschnig, I grab it from behind now, making sure the current behaviour is unchanged. The only use of this is in dependency_graph.h for indexing the post_dominator map. There was another use in remove returns that was unnecessary (see below).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was probably my doing/my fault. Can we just get rid of it completely instead? I'm not even sure that initialize method of dependence_grapht is ever called. If it is, let's add another parameter that provides the function name.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

initialize overrides ait::initialize...

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have

  virtual void initialize(const goto_programt &);
  virtual void initialize(const goto_functionst::goto_functiont &);

-> #1848.

}

template <typename Target>
Expand Down Expand Up @@ -521,6 +512,22 @@ class goto_program_templatet
}
}

/// Sets the `function` member of each instruction if not yet set
/// Note that a goto program need not be a goto function and therefore,
/// we cannot do this in update(), but only at the level of
/// of goto_functionst where goto programs are guaranteed to be
/// named functions.
void update_instructions_function(const irep_idt &function_id)
{
for(auto &instruction : instructions)
{
if(instruction.function.empty())
{
instruction.function = function_id;
}
}
}

/// Compute location numbers
void compute_location_numbers()
{
Expand Down
3 changes: 1 addition & 2 deletions src/goto-programs/remove_returns.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -244,8 +244,7 @@ void remove_returnst::operator()(
if(goto_function.body.empty())
return;

replace_returns(
goto_programt::get_function_id(goto_function.body), goto_function);
replace_returns(model_function.get_function_id(), goto_function);
do_function_calls(function_is_stub, goto_function.body);
}

Expand Down
3 changes: 3 additions & 0 deletions src/jbmc/jbmc_parse_options.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -708,6 +708,9 @@ void jbmc_parse_optionst::process_goto_function(
symbol_table.lookup_ref(new_symbol_name),
symbol_table);
}

// update the function member in each instruction
function.update_instructions_function();
}

catch(const char *e)
Expand Down