[RFC] LONGEST and ULONGEST function template instantiation
Pedro Alves
palves@redhat.com
Wed Jun 14 16:12:00 GMT 2017
On 06/13/2017 12:29 PM, Yao Qi wrote:
> This patch converts functions extract_{unsigned,signed}_integer
> to a function template extract_integer, which has two instantiations. It
> also does the similar changes to store__{unsigned,signed}_integer,
> regcache::raw_read_{unsigned,signed}, regcache::raw_write_{unsigned,signed},
> regcache::cooked_read_{unsigned,signed},
> regcache::cooked_write_{unsigned,signed}. It is an RFC, because I
> am not sure people like it or hate it :)
I like it, but I'd like it more with a few tweaks to the
implementation. :-) See patch below.
>
> This patch was posted here
> https://sourceware.org/ml/gdb-patches/2017-05/msg00492.html but the
> problem was fixed in a different way. However, I think the patch is still
> useful to shorten the code.
>
> The patch increase the binary code size (measured on x86_64-linux), from
>
> $ size ./gdb
> text data bss dec hex filename
> 23856767 2395936 364616 26617319 19625e7 ./gdb
>
> to
>
> $ size ./gdb
> text data bss dec hex filename
> 23858667 2395936 364616 26619219 1962d53 ./gdb
The version below has no impact on code size:
$ size gdb.before gdb.after
text data bss dec hex filename
7535236 125008 181184 7841428 77a694 gdb.before
7535236 125008 181184 7841428 77a694 gdb.after
Since the templates will ever only be instantiated with T=LONGEST
and T=ULONGEST, then there's no actual need to move the
templates' definitions to the header file.
Even if we wanted to move the functions to the header so that
their bodies can be inlined for performance (no idea whether it
really helps anything), I'd still think this is a useful
intermediate step, since it avoids both changing and moving the
function bodies at the same time.
> +
> +#define extract_unsigned_integer extract_integer<ULONGEST>
> +
> +#define extract_signed_integer extract_integer<LONGEST>
These can be static inline functions, instead of macros.
On 06/14/2017 02:54 PM, Alan Hayward wrote:
> My only objection to this would be that “enable_if” is fairly confusing.
> However, having said that, I think the end result is quite nice, and
> calling the functions is very simple, so I withdraw my objection.
>
> It would be nice if the header files could use “LongType” instead of
> the longer “enable_if” block, however I remember when I tried this
> myself and couldnÂ’t find a way around it.
>
In C++11 we have default template params, so we can use those for the
SFINAE magic instead of having to use a function's parameter of the
function's return type. Also, I think that using the "gdb::Requires"
helper makes the parameter's intention clearer.
>From bce7ebe33950b5a2cd5343d9a678ad23f02ec625 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Wed, 14 Jun 2017 16:24:07 +0100
Subject: [PATCH] extract/store integer templates
---
gdb/defs.h | 47 +++++++++++++++++++-----
gdb/findvar.c | 105 ++++++++++++++++++------------------------------------
gdb/regcache.c | 108 +++++++++++++-------------------------------------------
gdb/regcache.h | 20 +++++------
gdb/sh64-tdep.c | 12 +++----
5 files changed, 113 insertions(+), 179 deletions(-)
diff --git a/gdb/defs.h b/gdb/defs.h
index a1a97bb..55d16d1 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -82,6 +82,11 @@ enum compile_i_scope_types
COMPILE_I_PRINT_VALUE_SCOPE,
};
+
+template<typename T>
+using RequireLongest = gdb::Requires<gdb::Or<std::is_same<T, LONGEST>,
+ std::is_same<T, ULONGEST>>>;
+
/* Just in case they're not defined in stdio.h. */
#ifndef SEEK_SET
@@ -637,11 +642,22 @@ enum { MAX_REGISTER_SIZE = 64 };
/* In findvar.c. */
-extern LONGEST extract_signed_integer (const gdb_byte *, int,
- enum bfd_endian);
+template<typename T, typename = RequireLongest<T>>
+T extract_integer (const gdb_byte *addr, int len, enum bfd_endian byte_order);
-extern ULONGEST extract_unsigned_integer (const gdb_byte *, int,
- enum bfd_endian);
+static inline LONGEST
+extract_signed_integer (const gdb_byte *addr, int len,
+ enum bfd_endian byte_order)
+{
+ return extract_integer<LONGEST> (addr, len, byte_order);
+}
+
+static inline ULONGEST
+extract_unsigned_integer (const gdb_byte *addr, int len,
+ enum bfd_endian byte_order)
+{
+ return extract_integer<ULONGEST> (addr, len, byte_order);
+}
extern int extract_long_unsigned_integer (const gdb_byte *, int,
enum bfd_endian, LONGEST *);
@@ -649,11 +665,26 @@ extern int extract_long_unsigned_integer (const gdb_byte *, int,
extern CORE_ADDR extract_typed_address (const gdb_byte *buf,
struct type *type);
-extern void store_signed_integer (gdb_byte *, int,
- enum bfd_endian, LONGEST);
+/* All 'store' functions accept a host-format integer and store a
+ target-format integer at ADDR which is LEN bytes long. */
-extern void store_unsigned_integer (gdb_byte *, int,
- enum bfd_endian, ULONGEST);
+template<typename T, typename = RequireLongest<T>>
+extern void store_integer (gdb_byte *addr, int len, enum bfd_endian byte_order,
+ T val);
+
+static inline void
+store_signed_integer (gdb_byte *addr, int len,
+ enum bfd_endian byte_order, LONGEST val)
+{
+ return store_integer (addr, len, byte_order, val);
+}
+
+static inline void
+store_unsigned_integer (gdb_byte *addr, int len,
+ enum bfd_endian byte_order, ULONGEST val)
+{
+ return store_integer (addr, len, byte_order, val);
+}
extern void store_typed_address (gdb_byte *buf, struct type *type,
CORE_ADDR addr);
diff --git a/gdb/findvar.c b/gdb/findvar.c
index 6c18e25..beb127e 100644
--- a/gdb/findvar.c
+++ b/gdb/findvar.c
@@ -47,70 +47,54 @@
you lose
#endif
-LONGEST
-extract_signed_integer (const gdb_byte *addr, int len,
- enum bfd_endian byte_order)
+template<typename T, typename>
+T
+extract_integer (const gdb_byte *addr, int len, enum bfd_endian byte_order)
{
- LONGEST retval;
+ T retval = 0;
const unsigned char *p;
const unsigned char *startaddr = addr;
const unsigned char *endaddr = startaddr + len;
- if (len > (int) sizeof (LONGEST))
+ if (len > (int) sizeof (T))
error (_("\
That operation is not available on integers of more than %d bytes."),
- (int) sizeof (LONGEST));
+ (int) sizeof (T));
/* Start at the most significant end of the integer, and work towards
the least significant. */
if (byte_order == BFD_ENDIAN_BIG)
{
p = startaddr;
- /* Do the sign extension once at the start. */
- retval = ((LONGEST) * p ^ 0x80) - 0x80;
- for (++p; p < endaddr; ++p)
+ if (std::is_signed<T>::value)
+ {
+ /* Do the sign extension once at the start. */
+ retval = ((LONGEST) * p ^ 0x80) - 0x80;
+ ++p;
+ }
+ for (; p < endaddr; ++p)
retval = (retval << 8) | *p;
}
else
{
p = endaddr - 1;
- /* Do the sign extension once at the start. */
- retval = ((LONGEST) * p ^ 0x80) - 0x80;
- for (--p; p >= startaddr; --p)
+ if (std::is_signed<T>::value)
+ {
+ /* Do the sign extension once at the start. */
+ retval = ((LONGEST) * p ^ 0x80) - 0x80;
+ --p;
+ }
+ for (; p >= startaddr; --p)
retval = (retval << 8) | *p;
}
return retval;
}
-ULONGEST
-extract_unsigned_integer (const gdb_byte *addr, int len,
- enum bfd_endian byte_order)
-{
- ULONGEST retval;
- const unsigned char *p;
- const unsigned char *startaddr = addr;
- const unsigned char *endaddr = startaddr + len;
-
- if (len > (int) sizeof (ULONGEST))
- error (_("\
-That operation is not available on integers of more than %d bytes."),
- (int) sizeof (ULONGEST));
-
- /* Start at the most significant end of the integer, and work towards
- the least significant. */
- retval = 0;
- if (byte_order == BFD_ENDIAN_BIG)
- {
- for (p = startaddr; p < endaddr; ++p)
- retval = (retval << 8) | *p;
- }
- else
- {
- for (p = endaddr - 1; p >= startaddr; --p)
- retval = (retval << 8) | *p;
- }
- return retval;
-}
+/* Explicit instantiations. */
+template LONGEST extract_integer<LONGEST> (const gdb_byte *addr, int len,
+ enum bfd_endian byte_order);
+template ULONGEST extract_integer<ULONGEST> (const gdb_byte *addr, int len,
+ enum bfd_endian byte_order);
/* Sometimes a long long unsigned integer can be extracted as a
LONGEST value. This is done so that we can print these values
@@ -180,10 +164,10 @@ extract_typed_address (const gdb_byte *buf, struct type *type)
/* All 'store' functions accept a host-format integer and store a
target-format integer at ADDR which is LEN bytes long. */
-
+template<typename T, typename>
void
-store_signed_integer (gdb_byte *addr, int len,
- enum bfd_endian byte_order, LONGEST val)
+store_integer (gdb_byte *addr, int len, enum bfd_endian byte_order,
+ T val)
{
gdb_byte *p;
gdb_byte *startaddr = addr;
@@ -209,33 +193,14 @@ store_signed_integer (gdb_byte *addr, int len,
}
}
-void
-store_unsigned_integer (gdb_byte *addr, int len,
- enum bfd_endian byte_order, ULONGEST val)
-{
- unsigned char *p;
- unsigned char *startaddr = (unsigned char *) addr;
- unsigned char *endaddr = startaddr + len;
+/* Explicit instantiations. */
+template void store_integer (gdb_byte *addr, int len,
+ enum bfd_endian byte_order,
+ LONGEST val);
- /* Start at the least significant end of the integer, and work towards
- the most significant. */
- if (byte_order == BFD_ENDIAN_BIG)
- {
- for (p = endaddr - 1; p >= startaddr; --p)
- {
- *p = val & 0xff;
- val >>= 8;
- }
- }
- else
- {
- for (p = startaddr; p < endaddr; ++p)
- {
- *p = val & 0xff;
- val >>= 8;
- }
- }
-}
+template void store_integer (gdb_byte *addr, int len,
+ enum bfd_endian byte_order,
+ ULONGEST val);
/* Store the address ADDR as a pointer of type TYPE at BUF, in target
form. */
diff --git a/gdb/regcache.c b/gdb/regcache.c
index 46f4641..608717f 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -662,11 +662,12 @@ enum register_status
regcache_raw_read_signed (struct regcache *regcache, int regnum, LONGEST *val)
{
gdb_assert (regcache != NULL);
- return regcache->raw_read_signed (regnum, val);
+ return regcache->raw_read (regnum, val);
}
+template<typename T, typename>
enum register_status
-regcache::raw_read_signed (int regnum, LONGEST *val)
+regcache::raw_read (int regnum, T *val)
{
gdb_byte *buf;
enum register_status status;
@@ -675,9 +676,9 @@ regcache::raw_read_signed (int regnum, LONGEST *val)
buf = (gdb_byte *) alloca (m_descr->sizeof_register[regnum]);
status = raw_read (regnum, buf);
if (status == REG_VALID)
- *val = extract_signed_integer
- (buf, m_descr->sizeof_register[regnum],
- gdbarch_byte_order (m_descr->gdbarch));
+ *val = extract_integer<T> (buf,
+ m_descr->sizeof_register[regnum],
+ gdbarch_byte_order (m_descr->gdbarch));
else
*val = 0;
return status;
@@ -688,44 +689,26 @@ regcache_raw_read_unsigned (struct regcache *regcache, int regnum,
ULONGEST *val)
{
gdb_assert (regcache != NULL);
- return regcache->raw_read_unsigned (regnum, val);
-}
-
-
-enum register_status
-regcache::raw_read_unsigned (int regnum, ULONGEST *val)
-{
- gdb_byte *buf;
- enum register_status status;
-
- gdb_assert (regnum >= 0 && regnum < m_descr->nr_raw_registers);
- buf = (gdb_byte *) alloca (m_descr->sizeof_register[regnum]);
- status = raw_read (regnum, buf);
- if (status == REG_VALID)
- *val = extract_unsigned_integer
- (buf, m_descr->sizeof_register[regnum],
- gdbarch_byte_order (m_descr->gdbarch));
- else
- *val = 0;
- return status;
+ return regcache->raw_read (regnum, val);
}
void
regcache_raw_write_signed (struct regcache *regcache, int regnum, LONGEST val)
{
gdb_assert (regcache != NULL);
- regcache->raw_write_signed (regnum, val);
+ regcache->raw_write (regnum, val);
}
+template<typename T, typename>
void
-regcache::raw_write_signed (int regnum, LONGEST val)
+regcache::raw_write (int regnum, T val)
{
gdb_byte *buf;
gdb_assert (regnum >=0 && regnum < m_descr->nr_raw_registers);
buf = (gdb_byte *) alloca (m_descr->sizeof_register[regnum]);
- store_signed_integer (buf, m_descr->sizeof_register[regnum],
- gdbarch_byte_order (m_descr->gdbarch), val);
+ store_integer (buf, m_descr->sizeof_register[regnum],
+ gdbarch_byte_order (m_descr->gdbarch), val);
raw_write (regnum, buf);
}
@@ -734,19 +717,7 @@ regcache_raw_write_unsigned (struct regcache *regcache, int regnum,
ULONGEST val)
{
gdb_assert (regcache != NULL);
- regcache->raw_write_unsigned (regnum, val);
-}
-
-void
-regcache::raw_write_unsigned (int regnum, ULONGEST val)
-{
- gdb_byte *buf;
-
- gdb_assert (regnum >=0 && regnum < m_descr->nr_raw_registers);
- buf = (gdb_byte *) alloca (m_descr->sizeof_register[regnum]);
- store_unsigned_integer (buf, m_descr->sizeof_register[regnum],
- gdbarch_byte_order (m_descr->gdbarch), val);
- raw_write (regnum, buf);
+ regcache->raw_write (regnum, val);
}
LONGEST
@@ -857,11 +828,12 @@ regcache_cooked_read_signed (struct regcache *regcache, int regnum,
LONGEST *val)
{
gdb_assert (regcache != NULL);
- return regcache->cooked_read_signed (regnum, val);
+ return regcache->cooked_read (regnum, val);
}
+template<typename T, typename>
enum register_status
-regcache::cooked_read_signed (int regnum, LONGEST *val)
+regcache::cooked_read (int regnum, T *val)
{
enum register_status status;
gdb_byte *buf;
@@ -870,9 +842,8 @@ regcache::cooked_read_signed (int regnum, LONGEST *val)
buf = (gdb_byte *) alloca (m_descr->sizeof_register[regnum]);
status = cooked_read (regnum, buf);
if (status == REG_VALID)
- *val = extract_signed_integer
- (buf, m_descr->sizeof_register[regnum],
- gdbarch_byte_order (m_descr->gdbarch));
+ *val = extract_integer<T> (buf, m_descr->sizeof_register[regnum],
+ gdbarch_byte_order (m_descr->gdbarch));
else
*val = 0;
return status;
@@ -883,25 +854,7 @@ regcache_cooked_read_unsigned (struct regcache *regcache, int regnum,
ULONGEST *val)
{
gdb_assert (regcache != NULL);
- return regcache->cooked_read_unsigned (regnum, val);
-}
-
-enum register_status
-regcache::cooked_read_unsigned (int regnum, ULONGEST *val)
-{
- enum register_status status;
- gdb_byte *buf;
-
- gdb_assert (regnum >= 0 && regnum < m_descr->nr_cooked_registers);
- buf = (gdb_byte *) alloca (m_descr->sizeof_register[regnum]);
- status = cooked_read (regnum, buf);
- if (status == REG_VALID)
- *val = extract_unsigned_integer
- (buf, m_descr->sizeof_register[regnum],
- gdbarch_byte_order (m_descr->gdbarch));
- else
- *val = 0;
- return status;
+ return regcache->cooked_read (regnum, val);
}
void
@@ -909,18 +862,19 @@ regcache_cooked_write_signed (struct regcache *regcache, int regnum,
LONGEST val)
{
gdb_assert (regcache != NULL);
- regcache->cooked_write_signed (regnum, val);
+ regcache->cooked_write (regnum, val);
}
+template<typename T, typename>
void
-regcache::cooked_write_signed (int regnum, LONGEST val)
+regcache::cooked_write (int regnum, T val)
{
gdb_byte *buf;
gdb_assert (regnum >=0 && regnum < m_descr->nr_cooked_registers);
buf = (gdb_byte *) alloca (m_descr->sizeof_register[regnum]);
- store_signed_integer (buf, m_descr->sizeof_register[regnum],
- gdbarch_byte_order (m_descr->gdbarch), val);
+ store_integer (buf, m_descr->sizeof_register[regnum],
+ gdbarch_byte_order (m_descr->gdbarch), val);
cooked_write (regnum, buf);
}
@@ -929,19 +883,7 @@ regcache_cooked_write_unsigned (struct regcache *regcache, int regnum,
ULONGEST val)
{
gdb_assert (regcache != NULL);
- regcache->cooked_write_unsigned (regnum, val);
-}
-
-void
-regcache::cooked_write_unsigned (int regnum, ULONGEST val)
-{
- gdb_byte *buf;
-
- gdb_assert (regnum >=0 && regnum < m_descr->nr_cooked_registers);
- buf = (gdb_byte *) alloca (m_descr->sizeof_register[regnum]);
- store_unsigned_integer (buf, m_descr->sizeof_register[regnum],
- gdbarch_byte_order (m_descr->gdbarch), val);
- cooked_write (regnum, buf);
+ regcache->cooked_write (regnum, val);
}
/* See regcache.h. */
diff --git a/gdb/regcache.h b/gdb/regcache.h
index 4cf27a0..3f3f823 100644
--- a/gdb/regcache.h
+++ b/gdb/regcache.h
@@ -282,23 +282,19 @@ public:
#endif
void raw_write (int regnum, const gdb_byte *buf);
- enum register_status raw_read_signed (int regnum, LONGEST *val);
+ template<typename T, typename = RequireLongest<T>>
+ enum register_status raw_read (int regnum, T *val);
- void raw_write_signed (int regnum, LONGEST val);
-
- enum register_status raw_read_unsigned (int regnum, ULONGEST *val);
-
- void raw_write_unsigned (int regnum, ULONGEST val);
+ template<typename T, typename = RequireLongest<T>>
+ void raw_write (int regnum, T val);
struct value *cooked_read_value (int regnum);
- enum register_status cooked_read_signed (int regnum, LONGEST *val);
-
- void cooked_write_signed (int regnum, LONGEST val);
-
- enum register_status cooked_read_unsigned (int regnum, ULONGEST *val);
+ template<typename T, typename = RequireLongest<T>>
+ enum register_status cooked_read (int regnum, T *val);
- void cooked_write_unsigned (int regnum, ULONGEST val);
+ template<typename T, typename = RequireLongest<T>>
+ void cooked_write (int regnum, T val);
void raw_update (int regnum);
diff --git a/gdb/sh64-tdep.c b/gdb/sh64-tdep.c
index 4eb8947..3273197 100644
--- a/gdb/sh64-tdep.c
+++ b/gdb/sh64-tdep.c
@@ -1665,11 +1665,11 @@ sh64_pseudo_register_read (struct gdbarch *gdbarch, struct regcache *regcache,
*/
/* *INDENT-ON* */
/* Get FPSCR as an int. */
- status = regcache->raw_read_unsigned (fpscr_base_regnum, &fpscr_value);
+ status = regcache->raw_read (fpscr_base_regnum, &fpscr_value);
if (status != REG_VALID)
return status;
/* Get SR as an int. */
- status = regcache->raw_read_unsigned (sr_base_regnum, &sr_value);
+ status = regcache->raw_read (sr_base_regnum, &sr_value);
if (status != REG_VALID)
return status;
/* Build the new value. */
@@ -1847,15 +1847,15 @@ sh64_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache,
fpscr_value = fpscr_c_value & fpscr_mask;
sr_value = (fpscr_value & sr_mask) >> 6;
- regcache->raw_read_unsigned (fpscr_base_regnum, &old_fpscr_value);
+ regcache->raw_read (fpscr_base_regnum, &old_fpscr_value);
old_fpscr_value &= 0xfffc0002;
fpscr_value |= old_fpscr_value;
- regcache->raw_write_unsigned (fpscr_base_regnum, fpscr_value);
+ regcache->raw_write (fpscr_base_regnum, fpscr_value);
- regcache->raw_read_unsigned (sr_base_regnum, &old_sr_value);
+ regcache->raw_read (sr_base_regnum, &old_sr_value);
old_sr_value &= 0xffff8fff;
sr_value |= old_sr_value;
- regcache->raw_write_unsigned (sr_base_regnum, sr_value);
+ regcache->raw_write (sr_base_regnum, sr_value);
}
else if (reg_nr == FPUL_C_REGNUM)
--
2.5.5
More information about the Gdb-patches
mailing list