[binutils-gdb] gdb/riscv: Create each unique target description only once

[Andrew Burgess <aburgess at sourceware dot org>]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=634494366c515a89c4747d8a68a8da9218bb4969

commit 634494366c515a89c4747d8a68a8da9218bb4969
Author: Andrew Burgess <andrew.burgess@embecosm.com>
Date:   Thu Nov 29 15:51:58 2018 +0000

    gdb/riscv: Create each unique target description only once
    
    GDB relies on the fact that if two target descriptions have the same
    contents, then they will be the same object instance (having the same
    address).  One place where this is a requirement is in
    GDBARCH_LIST_LOOKUP_BY_INFO which is used to find previously created
    gdbarch objects.
    
    In GDBARCH_LIST_LOOKUP_BY_INFO a pointer comparison is made on the
    gdbarch's target description, if the pointers are different then it is
    assumed the gdbarches have different, non-compatible target
    descriptions.
    
    Previously we would create duplicate target descriptions in the belief
    that RISCV_GDBARCH_INIT would spot this duplication and discard the
    second instance.  However, this was incorrect, and instead we ended up
    creating duplicate gdbarch objects.
    
    With this commit every unique feature set will create one and only one
    target description, the feature set and resulting target description
    is then cached so that the same target description object can be
    returned later.
    
    Many other target avoid this problem by creating a small number of
    named target descriptions, and returning one of these.  However, we
    currently have 8 possible target descriptions (32 vs 64 bit for x-reg
    and f-reg, and h/w or s/w float abi) and creating each of these just
    to avoid a dynamic cache seems pointless.
    
    gdb/ChangeLog:
    
    	* arch/riscv.h (riscv_gdbarch_features::hash): New method.
    	* arch/riscv.c (struct riscv_gdbarch_features_hasher): New.
    	(riscv_tdesc_cache): New global.
    	(riscv_create_target_description): Look in the cache before
    	creating a new target description.

Diff:
---
 gdb/ChangeLog    |  8 ++++++++
 gdb/arch/riscv.c | 31 +++++++++++++++++++++++++++++++
 gdb/arch/riscv.h |  9 +++++++++
 3 files changed, 48 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 306e1a1..666f078 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,13 @@
 2018-11-30  Andrew Burgess  <andrew.burgess@embecosm.com>
 
+	* arch/riscv.h (riscv_gdbarch_features::hash): New method.
+	* arch/riscv.c (struct riscv_gdbarch_features_hasher): New.
+	(riscv_tdesc_cache): New global.
+	(riscv_create_target_description): Look in the cache before
+	creating a new target description.
+
+2018-11-30  Andrew Burgess  <andrew.burgess@embecosm.com>
+
 	* arch/riscv.h (riscv_gdb_features::operator==): New.
 	(riscv_gdb_features::operator!=): New.
 	* riscv-tdep.c (riscv_gdbarch_init): Make use of the inequality
diff --git a/gdb/arch/riscv.c b/gdb/arch/riscv.c
index cb715fa..81fd2f2 100644
--- a/gdb/arch/riscv.c
+++ b/gdb/arch/riscv.c
@@ -18,17 +18,45 @@
 #include "common-defs.h"
 #include "riscv.h"
 #include <stdlib.h>
+#include <unordered_map>
 
 #include "../features/riscv/32bit-cpu.c"
 #include "../features/riscv/64bit-cpu.c"
 #include "../features/riscv/32bit-fpu.c"
 #include "../features/riscv/64bit-fpu.c"
 
+/* Wrapper used by std::unordered_map to generate hash for feature set.  */
+struct riscv_gdbarch_features_hasher
+{
+  std::size_t
+  operator() (const riscv_gdbarch_features &features) const noexcept
+  {
+    return features.hash ();
+  }
+};
+
+/* Cache of previously seen target descriptions, indexed by the feature set
+   that created them.  */
+static std::unordered_map<riscv_gdbarch_features,
+                          target_desc *,
+                          riscv_gdbarch_features_hasher> riscv_tdesc_cache;
+
 /* See arch/riscv.h.  */
 
 const target_desc *
 riscv_create_target_description (struct riscv_gdbarch_features features)
 {
+  /* Have we seen this feature set before?  If we have return the same
+     target description.  GDB expects that if two target descriptions are
+     the same (in content terms) then they will actually be the same
+     instance.  This is important when trying to lookup gdbarch objects as
+     GDBARCH_LIST_LOOKUP_BY_INFO performs a pointer comparison on target
+     descriptions to find candidate gdbarch objects.  */
+  const auto it = riscv_tdesc_cache.find (features);
+  if (it != riscv_tdesc_cache.end ())
+    return it->second;
+
+  /* Now we should create a new target description.  */
   target_desc *tdesc = allocate_target_description ();
 
 #ifndef IN_PROCESS_AGENT
@@ -65,5 +93,8 @@ riscv_create_target_description (struct riscv_gdbarch_features features)
   else if (features.flen == 8)
     regnum = create_feature_riscv_64bit_fpu (tdesc, regnum);
 
+  /* Add to the cache.  */
+  riscv_tdesc_cache.emplace (features, tdesc);
+
   return tdesc;
 }
diff --git a/gdb/arch/riscv.h b/gdb/arch/riscv.h
index be41828e..79f5ec6 100644
--- a/gdb/arch/riscv.h
+++ b/gdb/arch/riscv.h
@@ -66,6 +66,15 @@ struct riscv_gdbarch_features
   {
     return !((*this) == rhs);
   }
+
+  /* Used by std::unordered_map to hash feature sets.  */
+  std::size_t hash () const noexcept
+  {
+    std::size_t val = ((xlen & 0x1f) << 6
+                       | (flen & 0x1f) << 1
+                       | (hw_float_abi ? 1 : 0));
+    return val;
+  }
 };
 
 /* Create and return a target description that is compatible with