cfgt: use dense_integer_mapt to provide instruction -> cfg node mapping

This saves a lot of allocations, memory, and time when the cfg is frequently queried.

Author: smowton

src/goto-analyzer/unreachable_instructions.cpp

@@ -32,15 +32,11 @@ static void unreachable_instructions(
   cfg_dominatorst dominators;
   dominators(goto_program);
 
-  for(cfg_dominatorst::cfgt::entry_mapt::const_iterator
-      it=dominators.cfg.entry_map.begin();
-      it!=dominators.cfg.entry_map.end();
-      ++it)
+  for(const auto instruction : dominators.cfg.entry_map.keys())
   {
-    const cfg_dominatorst::cfgt::nodet &n=dominators.cfg[it->second];
+    const cfg_dominatorst::cfgt::nodet &n = dominators.get_node(instruction);
     if(n.dominators.empty())
-      dest.insert(std::make_pair(it->first->location_number,
-                                 it->first));
+      dest.insert(std::make_pair(instruction->location_number, instruction));
   }
 }
 

src/goto-instrument/dump_c.cpp

@@ -663,6 +663,9 @@ void dump_ct::cleanup_decl(
 
   tmp.add(goto_programt::make_end_function());
 
+  // goto_program2codet requires valid location numbers:
+  tmp.update();
+
   std::unordered_set<irep_idt> typedef_names;
   for(const auto &td : typedef_map)
     typedef_names.insert(td.first);

src/goto-instrument/full_slicer.cpp

@@ -46,13 +46,10 @@ void full_slicert::add_function_calls(
   goto_programt::const_targett begin_function=
     f_it->second.body.instructions.begin();
 
-  cfgt::entry_mapt::const_iterator entry=
-    cfg.entry_map.find(begin_function);
-  assert(entry!=cfg.entry_map.end());
+  const auto &entry = cfg.entry_map.at(begin_function);
 
-  for(cfgt::edgest::const_iterator
-      it=cfg[entry->second].in.begin();
-      it!=cfg[entry->second].in.end();
+  for(cfgt::edgest::const_iterator it = cfg[entry].in.begin();
+      it != cfg[entry].in.end();
       ++it)
     add_to_queue(queue, it->first, node.PC);
 }
@@ -135,11 +132,9 @@ void full_slicert::add_jumps(
     goto_programt::const_targett lex_succ=j.PC;
     for( ; !lex_succ->is_end_function(); ++lex_succ)
     {
-      cfgt::entry_mapt::const_iterator entry=
-        cfg.entry_map.find(lex_succ);
-      assert(entry!=cfg.entry_map.end());
+      const auto &entry = cfg.entry_map.at(lex_succ);
 
-      if(cfg[entry->second].node_required)
+      if(cfg[entry].node_required)
         break;
     }
     if(lex_succ->is_end_function())
@@ -170,13 +165,10 @@ void full_slicert::add_jumps(
           d_it!=j_PC_node.dominators.end();
           ++d_it)
       {
-        cfgt::entry_mapt::const_iterator entry=
-          cfg.entry_map.find(*d_it);
-        assert(entry!=cfg.entry_map.end());
-
-        if(cfg[entry->second].node_required)
+        const auto &entry = cfg.entry_map.at(*d_it);
+        if(cfg[entry].node_required)
         {
-          const irep_idt &id2 = cfg[entry->second].function_id;
+          const irep_idt &id2 = cfg[entry].function_id;
           INVARIANT(id==id2,
                     "goto/jump expected to be within a single function");
 
@@ -211,11 +203,8 @@ void full_slicert::fixedpoint(
   dep_node_to_cfg.reserve(dep_graph.size());
   for(dependence_grapht::node_indext i=0; i<dep_graph.size(); ++i)
   {
-    cfgt::entry_mapt::const_iterator entry=
-      cfg.entry_map.find(dep_graph[i].PC);
-    assert(entry!=cfg.entry_map.end());
-
-    dep_node_to_cfg.push_back(entry->second);
+    const auto &entry = cfg.entry_map.at(dep_graph[i].PC);
+    dep_node_to_cfg.push_back(entry);
   }
 
   // process queue until empty
@@ -289,28 +278,26 @@ void full_slicert::operator()(
   // declarations or dead instructions may be necessary as well
   decl_deadt decl_dead;
 
-  for(cfgt::entry_mapt::iterator
-      e_it=cfg.entry_map.begin();
-      e_it!=cfg.entry_map.end();
-      e_it++)
+  for(const auto &instruction : cfg.entry_map.keys())
   {
-    if(criterion(cfg[e_it->second].function_id, e_it->first))
-      add_to_queue(queue, e_it->second, e_it->first);
-    else if(implicit(e_it->first))
-      add_to_queue(queue, e_it->second, e_it->first);
+    const auto &instruction_node_index = cfg.entry_map.at(instruction);
+    if(criterion(cfg[instruction_node_index].function_id, instruction))
+      add_to_queue(queue, instruction_node_index, instruction);
+    else if(implicit(instruction))
+      add_to_queue(queue, instruction_node_index, instruction);
     else if(
-      (e_it->first->is_goto() && e_it->first->get_condition().is_true()) ||
-      e_it->first->is_throw())
-      jumps.push_back(e_it->second);
-    else if(e_it->first->is_decl())
+      (instruction->is_goto() && instruction->get_condition().is_true()) ||
+      instruction->is_throw())
+      jumps.push_back(instruction_node_index);
+    else if(instruction->is_decl())
     {
-      const auto &s = to_code_decl(e_it->first->code).symbol();
-      decl_dead[s.get_identifier()].push(e_it->second);
+      const auto &s = to_code_decl(instruction->code).symbol();
+      decl_dead[s.get_identifier()].push(instruction_node_index);
     }
-    else if(e_it->first->is_dead())
+    else if(instruction->is_dead())
     {
-      const auto &s = to_code_dead(e_it->first->code).symbol();
-      decl_dead[s.get_identifier()].push(e_it->second);
+      const auto &s = to_code_dead(instruction->code).symbol();
+      decl_dead[s.get_identifier()].push(instruction_node_index);
     }
   }
 

src/goto-instrument/goto_instrument_parse_options.cpp

@@ -669,6 +669,10 @@ int goto_instrument_parse_optionst::doit()
       // applied
       restore_returns(goto_model);
 
+      // dump_c (actually goto_program2code) requires valid instruction
+      // location numbers:
+      goto_model.goto_functions.update();
+
       if(cmdline.args.size()==2)
       {
         #ifdef _MSC_VER
@@ -1497,6 +1501,10 @@ void goto_instrument_parse_optionst::instrument_goto_program()
     do_indirect_call_and_rtti_removal();
 
     log.status() << "Performing a reachability slice" << messaget::eom;
+
+    // reachability_slicer requires that the model has unique location numbers:
+    goto_model.goto_functions.update();
+
     if(cmdline.isset("property"))
       reachability_slicer(goto_model, cmdline.get_values("property"));
     else
@@ -1523,7 +1531,11 @@ void goto_instrument_parse_optionst::instrument_goto_program()
     if(cmdline.isset("property"))
       property_slicer(goto_model, cmdline.get_values("property"));
     else
+    {
+      // full_slicer requires that the model has unique location numbers:
+      goto_model.goto_functions.update();
       full_slicer(goto_model);
+    }
   }
 
   // splice option
@@ -1544,6 +1556,9 @@ void goto_instrument_parse_optionst::instrument_goto_program()
   {
     do_indirect_call_and_rtti_removal();
 
+    // reachability_slicer requires that the model has unique location numbers:
+    goto_model.goto_functions.update();
+
     log.status() << "Slicing away initializations of unused global variables"
                  << messaget::eom;
     slice_global_inits(goto_model);
@@ -1572,6 +1587,8 @@ void goto_instrument_parse_optionst::instrument_goto_program()
 
     aggressive_slicer.doit();
 
+    goto_model.goto_functions.update();
+
     log.status() << "Performing a reachability slice" << messaget::eom;
     if(cmdline.isset("property"))
       reachability_slicer(goto_model, cmdline.get_values("property"));

src/goto-instrument/points_to.cpp

@@ -21,14 +21,9 @@ void points_tot::fixedpoint()
   {
     added=false;
 
-    for(cfgt::entry_mapt::iterator
-        e_it=cfg.entry_map.begin();
-        e_it!=cfg.entry_map.end();
-        e_it++)
-    {
-      if(transform(cfg[e_it->second]))
+    for(const auto &key : cfg.entry_map.keys())
+      if(transform(cfg[cfg.entry_map.at(key)]))
         added=true;
-    }
   }
   while(added);
 }

src/goto-instrument/reachability_slicer.cpp

@@ -39,12 +39,13 @@ reachability_slicert::get_sources(
   const slicing_criteriont &criterion)
 {
   std::vector<cfgt::node_indext> sources;
-  for(const auto &e_it : cfg.entry_map)
+  for(const auto &instruction : cfg.entry_map.keys())
   {
+    const auto cfg_node_index = cfg.entry_map.at(instruction);
     if(
-      criterion(cfg[e_it.second].function_id, e_it.first) ||
-      is_threaded(e_it.first))
-      sources.push_back(e_it.second);
+      criterion(cfg[cfg_node_index].function_id, instruction) ||
+      is_threaded(instruction))
+      sources.push_back(cfg_node_index);
   }
 
   if(sources.empty())
@@ -64,12 +65,9 @@ bool reachability_slicert::is_same_target(
 {
   // Avoid comparing iterators belonging to different functions, and therefore
   // different std::lists.
-  cfgt::entry_mapt::const_iterator it1_entry = cfg.entry_map.find(it1);
-  cfgt::entry_mapt::const_iterator it2_entry = cfg.entry_map.find(it2);
-  return it1_entry != cfg.entry_map.end() && it2_entry != cfg.entry_map.end() &&
-         cfg[it1_entry->second].function_id ==
-           cfg[it2_entry->second].function_id &&
-         it1 == it2;
+  const auto &it1_entry = cfg.entry_map.at(it1);
+  const auto &it2_entry = cfg.entry_map.at(it2);
+  return cfg[it1_entry].function_id == cfg[it2_entry].function_id && it1 == it2;
 }
 
 /// Perform backward depth-first search of the control-flow graph of the

src/goto-programs/cfg.h

@@ -12,8 +12,9 @@ Author: Daniel Kroening, [email protected]
 #ifndef CPROVER_GOTO_PROGRAMS_CFG_H
 #define CPROVER_GOTO_PROGRAMS_CFG_H
 
-#include <util/std_expr.h>
+#include <util/dense_integer_map.h>
 #include <util/graph.h>
+#include <util/std_expr.h>
 
 #include "goto_functions.h"
 
@@ -31,6 +32,29 @@ struct cfg_base_nodet:public graph_nodet<empty_edget>, public T
   I PC;
 };
 
+/// Functor to convert cfg nodes into dense integers, used by \ref cfg_baset.
+/// Default implementation: the identity function.
+template <class T>
+class cfg_instruction_to_dense_integert
+{
+public:
+  std::size_t operator()(T &&t) const
+  {
+    return std::forward<T>(identity_functort<T>{}(t));
+  }
+};
+
+/// GOTO-instruction to location number functor.
+template <>
+class cfg_instruction_to_dense_integert<goto_programt::const_targett>
+{
+public:
+  std::size_t operator()(const goto_programt::const_targett &t) const
+  {
+    return t->location_number;
+  }
+};
+
 /// A multi-procedural control flow graph (CFG) whose nodes store references to
 /// instructions in a GOTO program.
 ///
@@ -66,28 +90,16 @@ class cfg_baset:public grapht< cfg_base_nodet<T, I> >
 
   class entry_mapt final
   {
-    typedef std::map<goto_programt::const_targett, entryt> data_typet;
+    typedef dense_integer_mapt<
+      goto_programt::const_targett,
+      entryt,
+      cfg_instruction_to_dense_integert<goto_programt::const_targett>>
+      data_typet;
     data_typet data;
 
   public:
     grapht< cfg_base_nodet<T, I> > &container;
 
-    // NOLINTNEXTLINE(readability/identifiers)
-    typedef data_typet::iterator iterator;
-    // NOLINTNEXTLINE(readability/identifiers)
-    typedef data_typet::const_iterator const_iterator;
-
-    template <typename U>
-    const_iterator find(U &&u) const { return data.find(std::forward<U>(u)); }
-
-    iterator begin() { return data.begin(); }
-    const_iterator begin() const { return data.begin(); }
-    const_iterator cbegin() const { return data.cbegin(); }
-
-    iterator end() { return data.end(); }
-    const_iterator end() const { return data.end(); }
-    const_iterator cend() const { return data.cend(); }
-
     explicit entry_mapt(grapht< cfg_base_nodet<T, I> > &_container):
       container(_container)
     {
@@ -97,10 +109,10 @@ class cfg_baset:public grapht< cfg_base_nodet<T, I> >
     {
       auto e=data.insert(std::make_pair(t, 0));
 
-      if(e.second)
-        e.first->second=container.add_node();
+      if(e)
+        data.at(t) = container.add_node();
 
-      return e.first->second;
+      return data.at(t);
     }
 
     entryt &at(const goto_programt::const_targett &t)
@@ -111,6 +123,25 @@ class cfg_baset:public grapht< cfg_base_nodet<T, I> >
     {
       return data.at(t);
     }
+
+    std::size_t count(const goto_programt::const_targett &t) const
+    {
+      return data.count(t);
+    }
+
+    typedef data_typet::possible_keyst keyst;
+    const keyst &keys() const
+    {
+      // We always define exactly the keys the entry map was set up for, so
+      // data's possible key set is exactly our key set
+      return data.possible_keys();
+    }
+
+    template <class Iter>
+    void setup_for_keys(Iter begin, Iter end)
+    {
+      data.setup_for_keys(begin, end);
+    }
   };
   entry_mapt entry_map;
 
@@ -170,12 +201,30 @@ class cfg_baset:public grapht< cfg_base_nodet<T, I> >
   void operator()(
     const goto_functionst &goto_functions)
   {
+    std::vector<goto_programt::const_targett> possible_keys;
+    for(const auto &id_and_function : goto_functions.function_map)
+    {
+      const auto &instructions = id_and_function.second.body.instructions;
+      possible_keys.reserve(
+        possible_keys.size() +
+        std::distance(instructions.begin(), instructions.end()));
+      for(auto it = instructions.begin(); it != instructions.end(); ++it)
+        possible_keys.push_back(it);
+    }
+    entry_map.setup_for_keys(possible_keys.begin(), possible_keys.end());
     compute_edges(goto_functions);
   }
 
   void operator()(P &goto_program)
   {
     goto_functionst goto_functions;
+    std::vector<goto_programt::const_targett> possible_keys;
+    const auto &instructions = goto_program.instructions;
+    possible_keys.reserve(
+      std::distance(instructions.begin(), instructions.end()));
+    for(auto it = instructions.begin(); it != instructions.end(); ++it)
+      possible_keys.push_back(it);
+    entry_map.setup_for_keys(possible_keys.begin(), possible_keys.end());
     compute_edges(goto_functions, goto_program);
   }