More improvements and cleanups in IdString subsystem

- better use of "inline" keyword
- deprecate "sticky" IDs feature
- improve handling of empty ID
- add move constructor

Signed-off-by: Clifford Wolf <clifford@clifford.at>

3 files changed, 54 insertions, 43 deletions

diff --git a/kernel/rtlil.cc b/kernel/rtlil.cc
index fade0bc36..91b6a1d30 100644
--- a/kernel/rtlil.cc
+++ b/kernel/rtlil.cc
@@ -33,8 +33,10 @@ std::vector<int> RTLIL::IdString::global_refcount_storage_;
 std::vector<char*> RTLIL::IdString::global_id_storage_;
 dict<char*, int, hash_cstr_ops> RTLIL::IdString::global_id_index_;
 std::vector<int> RTLIL::IdString::global_free_idx_list_;
+#ifdef YOSYS_USE_STICKY_IDS
 int RTLIL::IdString::last_created_idx_[8];
 int RTLIL::IdString::last_created_idx_ptr_;
+#endif
 
 RTLIL::Const::Const()
 {
diff --git a/kernel/rtlil.h b/kernel/rtlil.h
index 37b5f984c..e771655d7 100644
--- a/kernel/rtlil.h
+++ b/kernel/rtlil.h
@@ -78,6 +78,7 @@ namespace RTLIL
 	{
 		#undef YOSYS_XTRACE_GET_PUT
 		#undef YOSYS_SORT_ID_FREE_LIST
+		#undef YOSYS_USE_STICKY_IDS
 
 		// the global id string cache
 
@@ -92,8 +93,10 @@ namespace RTLIL
 		static dict<char*, int, hash_cstr_ops> global_id_index_;
 		static std::vector<int> global_free_idx_list_;
 
+	#ifdef YOSYS_USE_STICKY_IDS
 		static int last_created_idx_ptr_;
 		static int last_created_idx_[8];
+	#endif
 
 		static inline void xtrace_db_dump()
 		{
@@ -110,12 +113,14 @@ namespace RTLIL
 
 		static inline void checkpoint()
 		{
+		#ifdef YOSYS_USE_STICKY_IDS
 			last_created_idx_ptr_ = 0;
 			for (int i = 0; i < 8; i++) {
 				if (last_created_idx_[i])
 					put_reference(last_created_idx_[i]);
 				last_created_idx_[i] = 0;
 			}
+		#endif
 		#ifdef YOSYS_SORT_ID_FREE_LIST
 			std::sort(global_free_idx_list_.begin(), global_free_idx_list_.end(), std::greater<int>());
 		#endif
@@ -123,36 +128,42 @@ namespace RTLIL
 
 		static inline int get_reference(int idx)
 		{
-			global_refcount_storage_.at(idx)++;
+			if (idx) {
+				global_refcount_storage_[idx]++;
 		#ifdef YOSYS_XTRACE_GET_PUT
-			if (yosys_xtrace) {
-				log("#X# GET-BY-INDEX '%s' (index %d, refcount %d)\n", global_id_storage_.at(idx), idx, global_refcount_storage_.at(idx));
-			}
+				if (yosys_xtrace)
+					log("#X# GET-BY-INDEX '%s' (index %d, refcount %d)\n", global_id_storage_.at(idx), idx, global_refcount_storage_.at(idx));
 		#endif
+			}
 			return idx;
 		}
 
-		static inline int get_reference(const char *p)
+		static int get_reference(const char *p)
 		{
 			log_assert(destruct_guard.ok);
 
-			if (p[0]) {
-				log_assert(p[1] != 0);
-				log_assert(p[0] == '$' || p[0] == '\\');
-			}
+			if (!p[0])
+				return 0;
+
+			log_assert(p[0] == '$' || p[0] == '\\');
+			log_assert(p[1] != 0);
 
 			auto it = global_id_index_.find((char*)p);
 			if (it != global_id_index_.end()) {
 				global_refcount_storage_.at(it->second)++;
 		#ifdef YOSYS_XTRACE_GET_PUT
-				if (yosys_xtrace) {
+				if (yosys_xtrace)
 					log("#X# GET-BY-NAME '%s' (index %d, refcount %d)\n", global_id_storage_.at(it->second), it->second, global_refcount_storage_.at(it->second));
-				}
 		#endif
 				return it->second;
 			}
 
 			if (global_free_idx_list_.empty()) {
+				if (global_id_storage_.empty()) {
+					global_refcount_storage_.push_back(0);
+					global_id_storage_.push_back((char*)"");
+					global_id_index_[global_id_storage_.back()] = 0;
+				}
 				log_assert(global_id_storage_.size() < 0x40000000);
 				global_free_idx_list_.push_back(global_id_storage_.size());
 				global_id_storage_.push_back(nullptr);
@@ -165,23 +176,25 @@ namespace RTLIL
 			global_id_index_[global_id_storage_.at(idx)] = idx;
 			global_refcount_storage_.at(idx)++;
 
-			// Avoid Create->Delete->Create pattern
-			if (last_created_idx_[last_created_idx_ptr_])
-				put_reference(last_created_idx_[last_created_idx_ptr_]);
-			last_created_idx_[last_created_idx_ptr_] = idx;
-			get_reference(last_created_idx_[last_created_idx_ptr_]);
-			last_created_idx_ptr_ = (last_created_idx_ptr_ + 1) & 7;
-
 			if (yosys_xtrace) {
 				log("#X# New IdString '%s' with index %d.\n", p, idx);
 				log_backtrace("-X- ", yosys_xtrace-1);
 			}
 
 		#ifdef YOSYS_XTRACE_GET_PUT
-			if (yosys_xtrace) {
+			if (yosys_xtrace)
 				log("#X# GET-BY-NAME '%s' (index %d, refcount %d)\n", global_id_storage_.at(idx), idx, global_refcount_storage_.at(idx));
-			}
 		#endif
+
+		#ifdef YOSYS_USE_STICKY_IDS
+			// Avoid Create->Delete->Create pattern
+			if (last_created_idx_[last_created_idx_ptr_])
+				put_reference(last_created_idx_[last_created_idx_ptr_]);
+			last_created_idx_[last_created_idx_ptr_] = idx;
+			get_reference(last_created_idx_[last_created_idx_ptr_]);
+			last_created_idx_ptr_ = (last_created_idx_ptr_ + 1) & 7;
+		#endif
+
 			return idx;
 		}
 
@@ -189,7 +202,7 @@ namespace RTLIL
 		{
 			// put_reference() may be called from destructors after the destructor of
 			// global_refcount_storage_ has been run. in this case we simply do nothing.
-			if (!destruct_guard.ok)
+			if (!destruct_guard.ok || !idx)
 				return;
 
 		#ifdef YOSYS_XTRACE_GET_PUT
@@ -198,11 +211,13 @@ namespace RTLIL
 			}
 		#endif
 
-			log_assert(global_refcount_storage_.at(idx) > 0);
+			int &refcount = global_refcount_storage_[idx];
 
-			if (--global_refcount_storage_.at(idx) != 0)
+			if (--refcount > 0)
 				return;
 
+			log_assert(refcount == 0);
+
 			if (yosys_xtrace) {
 				log("#X# Removed IdString '%s' with index %d.\n", global_id_storage_.at(idx), idx);
 				log_backtrace("-X- ", yosys_xtrace-1);
@@ -218,41 +233,42 @@ namespace RTLIL
 
 		int index_;
 
-		IdString() : index_(get_reference("")) { }
-		IdString(const char *str) : index_(get_reference(str)) { }
-		IdString(const IdString &str) : index_(get_reference(str.index_)) { }
-		IdString(const std::string &str) : index_(get_reference(str.c_str())) { }
-		~IdString() { put_reference(index_); }
+		inline IdString() : index_(0) { }
+		inline IdString(const char *str) : index_(get_reference(str)) { }
+		inline IdString(const IdString &str) : index_(get_reference(str.index_)) { }
+		inline IdString(IdString &&str) : index_(str.index_) { str.index_ = 0; }
+		inline IdString(const std::string &str) : index_(get_reference(str.c_str())) { }
+		inline ~IdString() { put_reference(index_); }
 
-		void operator=(const IdString &rhs) {
+		inline void operator=(const IdString &rhs) {
 			put_reference(index_);
 			index_ = get_reference(rhs.index_);
 		}
 
-		void operator=(const char *rhs) {
+		inline void operator=(const char *rhs) {
 			IdString id(rhs);
 			*this = id;
 		}
 
-		void operator=(const std::string &rhs) {
+		inline void operator=(const std::string &rhs) {
 			IdString id(rhs);
 			*this = id;
 		}
 
-		const char *c_str() const {
+		inline const char *c_str() const {
 			return global_id_storage_.at(index_);
 		}
 
-		std::string str() const {
+		inline std::string str() const {
 			return std::string(global_id_storage_.at(index_));
 		}
 
-		bool operator<(const IdString &rhs) const {
+		inline bool operator<(const IdString &rhs) const {
 			return index_ < rhs.index_;
 		}
 
-		bool operator==(const IdString &rhs) const { return index_ == rhs.index_; }
-		bool operator!=(const IdString &rhs) const { return index_ != rhs.index_; }
+		inline bool operator==(const IdString &rhs) const { return index_ == rhs.index_; }
+		inline bool operator!=(const IdString &rhs) const { return index_ != rhs.index_; }
 
 		// The methods below are just convenience functions for better compatibility with std::string.
 
diff --git a/kernel/yosys.cc b/kernel/yosys.cc
index 5a53f90fd..fc9bd96fe 100644
--- a/kernel/yosys.cc
+++ b/kernel/yosys.cc
@@ -510,10 +510,6 @@ void yosys_setup()
 	if(already_setup)
 		return;
 	already_setup = true;
-	// if there are already IdString objects then we have a global initialization order bug
-	IdString empty_id;
-	log_assert(empty_id.index_ == 0);
-	IdString::get_reference(empty_id.index_);
 
 	#ifdef WITH_PYTHON
 		PyImport_AppendInittab((char*)"libyosys", INIT_MODULE);
@@ -575,9 +571,6 @@ void yosys_shutdown()
 #ifdef WITH_PYTHON
 	Py_Finalize();
 #endif
-
-	IdString empty_id;
-	IdString::put_reference(empty_id.index_);
 }
 
 RTLIL::IdString new_id(std::string file, int line, std::string func)