Improve memory management in VHDL target

Previously the VHDL code generator managed memory for
the AST objects by requiring that each AST element be
responsible for deleting its children. The disadvantages
of this are that it's quite easy to accidentally leak
memory by forgetting to delete a child, and no AST pointers
may be shared by multiple parents (or we'd end up with
double-deletes) -- this results in unnecessary copies of
objects being made.

There's no real need for fine-grained memory management of
AST objects since once they're allocated they tend to
persist until the code generator is about to terminate, when
they should all be freed.

This patch provides a custom new/delete operator for
vhdl_element which logs the vhdl_element objects allocated
in a std::vector (after calling the default operator new).
Once the code generator is finished a single free_all_objects
call deletes all the AST objects in one go. The custom delete
operator is required so that we can still explicitly deallocate
vhdl_element objects before the code generator completes.

There are also some allocation statistics printed at the end
when -pdebug=1 is specified.

tgt-vhdl/state.cc

@@ -20,6 +20,7 @@
 
 #include "state.hh"
 #include "vhdl_syntax.hh"
+#include "vhdl_target.h"
 
 #include <algorithm>
 #include <string>
@@ -187,10 +188,12 @@ void emit_all_entities(std::ostream& os, int max_depth)
 // will be valid after this call.
 void free_all_vhdl_objects()
 {
-   for (entity_list_t::iterator it = g_entities.begin();
-        it != g_entities.end();
-        ++it)
-      delete (*it);
+   int freed = vhdl_element::free_all_objects();
+   debug_msg("Deallocated %d VHDL syntax objects", freed);
+
+   size_t total = vhdl_element::total_allocated();
+   debug_msg("%d total bytes used for VHDL syntax objects", total);
+   
    g_entities.clear();
 }
 

tgt-vhdl/vhdl_element.cc

@@ -27,6 +27,7 @@
 #include <iostream>
 #include <sstream>
 
+using namespace std;
 
 static const int VHDL_INDENT = 2;  // Spaces to indent
 
@@ -58,6 +59,13 @@ void blank_line(std::ostream &of, int level)
    newline(of, level);
 }
 
+// The array of all vhdl_elements allocated so we can quickly
+// clean them up just before the code generator exits
+vector<vhdl_element*> vhdl_element::allocated_;
+
+// Just a counter of total bytes allocated for statistics
+size_t vhdl_element::total_alloc_(0);
+
 void vhdl_element::set_comment(std::string comment)
 {
    comment_ = comment;
@@ -85,3 +93,71 @@ void vhdl_element::print() const
    emit(std::cout, 0);
    std::cout << std::endl;
 }
+
+// Trap allocations of vhdl_element subclasses.
+// This records the pointer allocated in a static field of vhdl_element
+// so we can delete it just before the code generator exits.
+void* vhdl_element::operator new(size_t size) throw (bad_alloc)
+{   
+   // Let the default new handle the allocation
+   void* ptr = ::operator new(size);
+
+   // Remember this element so we can delete it later
+   vhdl_element* elem = static_cast<vhdl_element*>(ptr);
+   allocated_.push_back(elem);
+
+   total_alloc_ += size;
+
+   return ptr;
+}
+
+// Explicitly delete a vhdl_element object.
+// This just sets the corresponding pointer in vhdl_element::allocated_
+// to NULL (since it's safe to delete a NULL pointer).
+void vhdl_element::operator delete(void* ptr)
+{   
+   // Let the default delete handle the deallocation
+   ::operator delete(ptr);
+
+   // Remember that we've already deleted this pointer so we don't
+   // delete it again in the call to free_all_objects
+   vector<vhdl_element*>::iterator it =
+      find(allocated_.begin(), allocated_.end(), static_cast<vhdl_element*>(ptr));
+
+   if (it != allocated_.end()) {
+      *it = NULL;   // It's safe to delete a NULL pointer and much cheaper
+                    // than removing an element from the middle of a vector
+   }
+   else {
+      // This shouldn't really happen but it's harmless
+      cerr << "??? vhdl_element::operator delete called on an object not "
+           << "allocated by vhdl_element::operator new" << endl;
+   }
+}
+
+// Return the total number of bytes our custom operator new has seen.
+size_t vhdl_element::total_allocated()
+{
+   return total_alloc_;
+}
+
+// Free every object derived from vhdl_element that has not yet been
+// explicitly deallocated.
+// Any pointers to vhdl_elements will be invalid after this call!
+// Returns the number of objects freed.
+int vhdl_element::free_all_objects()
+{
+   for (vector<vhdl_element*>::iterator it = allocated_.begin();
+        it != allocated_.end(); ++it) {
+      if (*it)
+         ::operator delete(*it);  // Explicitly use the default delete
+   }
+
+   int freed = allocated_.size();
+
+   // Just in case we want to allocated any more vhdl_element objects
+   allocated_.clear();
+
+   return freed;
+}
+

tgt-vhdl/vhdl_element.hh

@@ -21,28 +21,49 @@
 #ifndef INC_VHDL_ELEMENT_HH
 #define INC_VHDL_ELEMENT_HH
 
-#include <fstream>
+#include <iosfwd>
 #include <list>
 #include <string>
+#include <new>
+#include <vector>
 
 typedef std::list<std::string> string_list_t;
 
-/*
- * Any VHDL syntax element. Each element can also contain a comment.
- */
+// Any VHDL syntax element. Each element can also contain a comment.
+//
+// Memory management is handled specially for vhdl_element subclasses:
+// The vast majority of vhdl_elements will be created during code generation
+// and persist until after they have been printed, at which point *all*
+// vhdl_element objects should be destroyed. To support this all allocations
+// of vhdl_element subclasses call a special operator new which records
+// the pointer allocated so we can ensure that it is disposed of when
+// the code generator completes -- by free_all_objects.
+//
+// The two big advantages of this are that we don't have to worry about
+// memory leaks of vhdl_element objects, and we can freely share pointers
+// between different parts of the AST.
 class vhdl_element {
 public:
    virtual ~vhdl_element() {}
+
+   void* operator new(size_t size) throw (std::bad_alloc);
+   void operator delete(void* ptr);
    
    virtual void emit(std::ostream &of, int level=0) const = 0;
    void print() const;
 
    void set_comment(std::string comment);
+
+   static int free_all_objects();
+   static size_t total_allocated();
 protected:
    void emit_comment(std::ostream &of, int level,
                      bool end_of_line=false) const;
 private:
    std::string comment_;
+
+   static std::vector<vhdl_element*> allocated_;
+   static size_t total_alloc_;
 };
 
 typedef std::list<vhdl_element*> element_list_t;

tgt-vhdl/vhdl_helper.hh

@@ -48,15 +48,6 @@ void emit_children(std::ostream &of,
    }
 }
 
-template <class T>
-void delete_children(std::list<T*> &children)
-{
-   typename std::list<T*>::iterator it;
-   for (it = children.begin(); it != children.end(); ++it)
-      delete *it;
-   children.clear();
-}
-
 static inline char vl_to_vhdl_bit(char bit)
 {
    switch (bit) {

tgt-vhdl/vhdl_syntax.cc

@@ -35,7 +35,7 @@ vhdl_scope::vhdl_scope()
 
 vhdl_scope::~vhdl_scope()
 {
-   delete_children<vhdl_decl>(decls_);
+   
 }
 
 void vhdl_scope::set_initializing(bool i)
@@ -94,8 +94,8 @@ vhdl_entity::vhdl_entity(const char *name, vhdl_arch *arch, int depth__)
 }
 
 vhdl_entity::~vhdl_entity()
-{   
-   delete arch_;
+{
+   
 }
 
 void vhdl_entity::add_port(vhdl_port_decl *decl)
@@ -131,7 +131,7 @@ void vhdl_entity::emit(std::ostream &of, int level) const
 
 vhdl_arch::~vhdl_arch()
 {
-   delete_children<vhdl_conc_stmt>(stmts_);
+   
 }
 
 void vhdl_arch::add_stmt(vhdl_process *proc)
@@ -199,7 +199,7 @@ void vhdl_process::emit(std::ostream &of, int level) const
 
 stmt_container::~stmt_container()
 {
-   delete_children<vhdl_seq_stmt>(stmts_);
+   
 }
 
 void stmt_container::add_stmt(vhdl_seq_stmt *stmt)
@@ -232,11 +232,7 @@ vhdl_comp_inst::vhdl_comp_inst(const char *inst_name, const char *comp_name)
 
 vhdl_comp_inst::~vhdl_comp_inst()
 {
-   port_map_list_t::iterator it;
-   for (it = mapping_.begin(); it != mapping_.end(); ++it) {
-      delete (*it).expr;
-   }
-   mapping_.clear();
+   
 }
 
 void vhdl_comp_inst::map_port(const char *name, vhdl_expr *expr)
@@ -312,8 +308,7 @@ void vhdl_component_decl::emit(std::ostream &of, int level) const
 
 vhdl_wait_stmt::~vhdl_wait_stmt()
 {
-   if (expr_ != NULL)
-      delete expr_;
+   
 }
 
 void vhdl_wait_stmt::emit(std::ostream &of, int level) const
@@ -354,10 +349,7 @@ void vhdl_wait_stmt::emit(std::ostream &of, int level) const
 
 vhdl_decl::~vhdl_decl()
 {
-   if (type_ != NULL)
-      delete type_;
-   if (initial_ != NULL)
-      delete initial_;
+   
 }
 
 const vhdl_type *vhdl_decl::get_type() const
@@ -434,8 +426,7 @@ void vhdl_type_decl::emit(std::ostream &of, int level) const
 
 vhdl_expr::~vhdl_expr()
 {
-   if (type_ != NULL)
-      delete type_;
+   
 }
 
 void vhdl_expr_list::add_expr(vhdl_expr *e)
@@ -445,7 +436,7 @@ void vhdl_expr_list::add_expr(vhdl_expr *e)
 
 vhdl_expr_list::~vhdl_expr_list()
 {
-   delete_children<vhdl_expr>(exprs_);
+   
 }
 
 void vhdl_expr_list::emit(std::ostream &of, int level) const
@@ -473,8 +464,7 @@ void vhdl_pcall_stmt::emit(std::ostream &of, int level) const
 
 vhdl_var_ref::~vhdl_var_ref()
 {
-   if (slice_)
-      delete slice_;
+   
 }
 
 void vhdl_var_ref::set_slice(vhdl_expr *s, int w)
@@ -538,10 +528,7 @@ void vhdl_fcall::emit(std::ostream &of, int level) const
 
 vhdl_abstract_assign_stmt::~vhdl_abstract_assign_stmt()
 {
-   delete lhs_;
-   delete rhs_;
-   if (after_)
-      delete after_;
+   
 }
 
 void vhdl_nbassign_stmt::emit(std::ostream &of, int level) const
@@ -619,15 +606,7 @@ void vhdl_const_time::emit(std::ostream &of, int level) const
 
 vhdl_cassign_stmt::~vhdl_cassign_stmt()
 {
-   delete lhs_;
-   delete rhs_;
-
-   for (std::list<when_part_t>::const_iterator it = whens_.begin();
-        it != whens_.end();
-        ++it) {
-      delete (*it).value;
-      delete (*it).cond;
-   }
+   
 }
 
 void vhdl_cassign_stmt::add_condition(vhdl_expr *value, vhdl_expr *cond)
@@ -676,7 +655,7 @@ vhdl_if_stmt::vhdl_if_stmt(vhdl_expr *test)
 
 vhdl_if_stmt::~vhdl_if_stmt()
 {
-   delete test_;
+   
 }
 
 stmt_container *vhdl_if_stmt::add_elsif(vhdl_expr *test)
@@ -712,7 +691,7 @@ void vhdl_if_stmt::emit(std::ostream &of, int level) const
 
 vhdl_unaryop_expr::~vhdl_unaryop_expr()
 {
-   delete operand_;
+   
 }
 
 void vhdl_unaryop_expr::emit(std::ostream &of, int level) const
@@ -740,7 +719,7 @@ vhdl_binop_expr::vhdl_binop_expr(vhdl_expr *left, vhdl_binop_t op,
 
 vhdl_binop_expr::~vhdl_binop_expr()
 {
-   delete_children<vhdl_expr>(operands_);
+   
 }
 
 void vhdl_binop_expr::add_expr(vhdl_expr *e)
@@ -776,12 +755,7 @@ void vhdl_binop_expr::emit(std::ostream &of, int level) const
 
 vhdl_bit_spec_expr::~vhdl_bit_spec_expr()
 {
-   if (others_)
-      delete others_;
-
-   std::list<bit_map>::iterator it;
-   for (it = bits_.begin(); it != bits_.end(); ++it)
-      delete (*it).e;
+   
 }
 
 void vhdl_bit_spec_expr::add_bit(int bit, vhdl_expr *e)
@@ -813,7 +787,7 @@ void vhdl_bit_spec_expr::emit(std::ostream &of, int level) const
 
 vhdl_case_branch::~vhdl_case_branch()
 {
-   delete when_;
+   
 }
 
 void vhdl_case_branch::emit(std::ostream &of, int level) const
@@ -826,7 +800,7 @@ void vhdl_case_branch::emit(std::ostream &of, int level) const
 
 vhdl_case_stmt::~vhdl_case_stmt()
 {
-   delete test_;
+   
 }
 
 void vhdl_case_stmt::emit(std::ostream &of, int level) const
@@ -851,7 +825,7 @@ void vhdl_case_stmt::emit(std::ostream &of, int level) const
 
 vhdl_while_stmt::~vhdl_while_stmt()
 {
-   delete test_;
+   
 }
 
 void vhdl_while_stmt::emit(std::ostream &of, int level) const
@@ -871,8 +845,7 @@ void vhdl_loop_stmt::emit(std::ostream &of, int level) const
 
 vhdl_for_stmt::~vhdl_for_stmt()
 {
-   delete from_;
-   delete to_;
+   
 }
 
 void vhdl_for_stmt::emit(std::ostream &of, int level) const
@@ -930,17 +903,7 @@ void vhdl_param_decl::emit(std::ostream &of, int level) const
 
 vhdl_with_select_stmt::~vhdl_with_select_stmt()
 {
-   delete test_;
-   delete out_;
-
-   for (when_list_t::const_iterator it = whens_.begin();
-        it != whens_.end();
-        ++it) {
-      delete (*it).value;
-      delete (*it).cond;
-      if ((*it).delay)
-         delete (*it).delay;
-   }
+   
 }
 
 void vhdl_with_select_stmt::emit(std::ostream &of, int level) const