public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Andrew Burgess <andrew.burgess@embecosm.com>
To: gdb-patches@sourceware.org
Cc: jimw@sifive.com,	palmer@sifive.com,	jhb@FreeBSD.org,
	Andrew Burgess <andrew.burgess@embecosm.com>
Subject: [PATCH 3/4] gdb/riscv: Create each unique target description only once
Date: Thu, 29 Nov 2018 16:50:00 -0000	[thread overview]
Message-ID: <84ce42bd83dc3d67bd312c4c8772171dd5bf1182.1543509416.git.andrew.burgess@embecosm.com> (raw)
In-Reply-To: <cover.1543509416.git.andrew.burgess@embecosm.com>
In-Reply-To: <cover.1543509416.git.andrew.burgess@embecosm.com>

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.c (class riscv_target_desc_cache): New.
	(riscv_tdesc_cache): New global.
	(riscv_create_target_description): Look in the cache before
	creating a new target description.
---
 gdb/ChangeLog    |  7 +++++++
 gdb/arch/riscv.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 64 insertions(+)

diff --git a/gdb/arch/riscv.c b/gdb/arch/riscv.c
index cb715fabb1f..fef0c403e7b 100644
--- a/gdb/arch/riscv.c
+++ b/gdb/arch/riscv.c
@@ -24,11 +24,65 @@
 #include "../features/riscv/32bit-fpu.c"
 #include "../features/riscv/64bit-fpu.c"
 
+/* Class used to cache previously created RISC-V target descriptions.  */
+
+class riscv_target_desc_cache
+{
+public:
+
+  /* Lookup a previously created target description for FEATURES, or
+     return nullptr if no matching target description has ever been
+     created.  */
+  const target_desc *lookup (struct riscv_gdbarch_features features)
+  {
+    for (auto p : m_tdesc_list)
+      {
+        if (p.first == features)
+          return p.second;
+      }
+
+    return nullptr;
+  }
+
+  /* Add TDESC into the cache, a target description created to match
+     FEATURES.  */
+  void add (const struct riscv_gdbarch_features features,
+            const target_desc *tdesc)
+  {
+    feature_tdesc_pair p (features, tdesc);
+    m_tdesc_list.push_back (p);
+  }
+
+private:
+
+  /* Map from a feature set to the corresponding target description.  */
+  typedef std::pair<const struct riscv_gdbarch_features,
+                    const target_desc *> feature_tdesc_pair;
+
+  /* List of all target descriptions we've previously seen.  */
+  std::vector<feature_tdesc_pair> m_tdesc_list;
+};
+
+/* Cache of previously seen target descriptions, indexed by the feature set
+   that created them.  */
+static riscv_target_desc_cache 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 target_desc *prev_tdesc = riscv_tdesc_cache.lookup (features);
+  if (prev_tdesc != nullptr)
+    return prev_tdesc;
+
+  /* Now we should create a new target description.  */
   target_desc *tdesc = allocate_target_description ();
 
 #ifndef IN_PROCESS_AGENT
@@ -65,5 +119,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.add (features, tdesc);
+
   return tdesc;
 }
-- 
2.14.5

  reply	other threads:[~2018-11-29 16:50 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-28 22:50 [RFC] gdb/riscv: Add read_description method for riscv_linux_nat_target Andrew Burgess
2018-11-28 23:37 ` Jim Wilson
2018-11-29  2:23 ` Jim Wilson
2018-11-29 16:50   ` Andrew Burgess [this message]
2018-11-29 18:12     ` [PATCH 3/4] gdb/riscv: Create each unique target description only once Pedro Alves
2018-11-29 19:17       ` Andrew Burgess
2018-11-29 22:32       ` Andrew Burgess
2018-11-30 17:07         ` Pedro Alves
2018-11-29 16:50   ` [PATCH 2/4] gdb/riscv: Add equality operators to riscv_gdb_features Andrew Burgess
2018-11-29 16:50   ` [PATCH 0/4] Re: gdb/riscv: Add read_description method for riscv_linux_nat_target Andrew Burgess
2018-11-29 16:50   ` [PATCH 1/4] gdb/riscv: Make some target description functions constant Andrew Burgess
2018-11-29 16:50   ` [PATCH 4/4] gdb/riscv: Add read_description method for riscv_linux_nat_target Andrew Burgess
2018-11-29 22:22     ` Jim Wilson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=84ce42bd83dc3d67bd312c4c8772171dd5bf1182.1543509416.git.andrew.burgess@embecosm.com \
    --to=andrew.burgess@embecosm.com \
    --cc=gdb-patches@sourceware.org \
    --cc=jhb@FreeBSD.org \
    --cc=jimw@sifive.com \
    --cc=palmer@sifive.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).