This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] gnulib: import count-one-bits module and use it

Hi,

On 2/11/20 2:09 PM, Simon Marchi wrote:

For a fix I intend to submit, I would need a function that counts the
number of set bits in a word.  There is  __builtin_popcount that is
supported by gcc and clang, but there is also a gnulib module that wraps
that and provides a fallback for other compilers, so I think it would be
good to use it.

I also noticed that there is a bitcount function in arch/arm.c, so I
thought that as a first step I would replace that one with the gnulib
count-one-bits module.  This is what this patch does.

The gnulib module provides multiple functions, with various parameter
length (unsigned int, unsigned long int, unsigned long long int), I
chose the one that made sense for each call site based on the argument
type.


Thanks. One small nit below.



gnulib/ChangeLog:

	* update-gnulib.sh (IMPORTED_GNULIB_MODULES): Import
	count-one-bits module.
	* configure: Re-generate.
	* aclocal.m4: Re-generate.
	* Makefile.in: Re-generate.
	* import/count-one-bits.c: New file.
	* import/count-one-bits.h: New file.
	* import/Makefile.am: Re-generate.
	* import/Makefile.in: Re-generate.
	* import/m4/gnulib-cache.m4: Re-generate.
	* import/m4/gnulib-comp.m4: Re-generate.
	* import/m4/count-one-bits.m4: New file.

gdb/ChangeLog:

	* arm-tdep.c: Include count-one-bits.h.
	(cleanup_block_store_pc): Use count_one_bits.
	(cleanup_block_load_pc): Use count_one_bits.
	(arm_copy_block_xfer): Use count_one_bits.
	(thumb2_copy_block_xfer): Use count_one_bits.
	(thumb_copy_pop_pc_16bit): Use count_one_bits.
	* arch/arm-get-next-pcs.c: Include count-one-bits.h.
	(thumb_get_next_pcs_raw): Use count_one_bits.
	(arm_get_next_pcs_raw): Use count_one_bits_l.
	* arch/arm.c (bitcount): Remove.
	* arch/arm.h (bitcount): Remove.
---
  gdb/arch/arm-get-next-pcs.c        |   9 +-
  gdb/arch/arm.c                     |  11 ---
  gdb/arch/arm.h                     |   3 -
  gdb/arm-tdep.c                     |  12 +--
  gnulib/Makefile.in                 |   5 +-
  gnulib/aclocal.m4                  |   1 +
  gnulib/configure                   | 118 +++++++++++++------------
  gnulib/import/Makefile.am          |   9 ++
  gnulib/import/Makefile.in          |  50 ++++++-----
  gnulib/import/count-one-bits.c     |   7 ++
  gnulib/import/count-one-bits.h     | 136 +++++++++++++++++++++++++++++
  gnulib/import/m4/count-one-bits.m4 |  12 +++
  gnulib/import/m4/gnulib-cache.m4   |   2 +
  gnulib/import/m4/gnulib-comp.m4    |   5 ++
  gnulib/update-gnulib.sh            |   1 +
  15 files changed, 276 insertions(+), 105 deletions(-)
  create mode 100644 gnulib/import/count-one-bits.c
  create mode 100644 gnulib/import/count-one-bits.h
  create mode 100644 gnulib/import/m4/count-one-bits.m4

diff --git a/gdb/arch/arm-get-next-pcs.c b/gdb/arch/arm-get-next-pcs.c
index fc541332aabd..0c49a77245bf 100644
--- a/gdb/arch/arm-get-next-pcs.c
+++ b/gdb/arch/arm-get-next-pcs.c
@@ -22,6 +22,7 @@
  #include "gdbsupport/common-regcache.h"
  #include "arm.h"
  #include "arm-get-next-pcs.h"
+#include "count-one-bits.h"
/* See arm-get-next-pcs.h. */ @@ -408,8 +409,8 @@ thumb_get_next_pcs_raw (struct arm_get_next_pcs *self) /* Fetch the saved PC from the stack. It's stored above 
           all of the other registers.  */
-      unsigned long offset = bitcount (bits (inst1, 0, 7))
-			     * ARM_INT_REGISTER_SIZE;
+      unsigned long offset
+	= count_one_bits (bits (inst1, 0, 7)) * ARM_INT_REGISTER_SIZE;
        sp = regcache_raw_get_unsigned (regcache, ARM_SP_REGNUM);
        nextpc = self->ops->read_mem_uint (sp + offset, 4, byte_order);
      }
@@ -496,7 +497,7 @@ thumb_get_next_pcs_raw (struct arm_get_next_pcs *self)
  	      /* LDMIA or POP */
  	      if (!bit (inst2, 15))
  		load_pc = 0;
-	      offset = bitcount (inst2) * 4 - 4;
+	      offset = count_one_bits (inst2) * 4 - 4;
  	    }
  	  else if (!bit (inst1, 7) && bit (inst1, 8))
  	    {
@@ -864,7 +865,7 @@ arm_get_next_pcs_raw (struct arm_get_next_pcs *self)
  		    {
  		      /* up */
  		      unsigned long reglist = bits (this_instr, 0, 14);
-		      offset = bitcount (reglist) * 4;
+		      offset = count_one_bits_l (reglist) * 4;
  		      if (bit (this_instr, 24))		/* pre */
  			offset += 4;
  		    }
diff --git a/gdb/arch/arm.c b/gdb/arch/arm.c
index 60d9f85889db..faa2b4fbd4b2 100644
--- a/gdb/arch/arm.c
+++ b/gdb/arch/arm.c
@@ -41,17 +41,6 @@ thumb_insn_size (unsigned short inst1)
/* See arm.h. */ -int 
-bitcount (unsigned long val)
-{
-  int nbits;
-  for (nbits = 0; val != 0; nbits++)
-    val &= val - 1;		/* Delete rightmost 1-bit in val.  */
-  return nbits;
-}
-
-/* See arm.h.  */
-
  int
  condition_true (unsigned long cond, unsigned long status_reg)
  {
diff --git a/gdb/arch/arm.h b/gdb/arch/arm.h
index 13f030af82ca..2bffb11a4a3f 100644
--- a/gdb/arch/arm.h
+++ b/gdb/arch/arm.h
@@ -159,9 +159,6 @@ int thumb_insn_size (unsigned short inst1);
  /* Returns true if the condition evaluates to true.  */
  int condition_true (unsigned long cond, unsigned long status_reg);
-/* Return number of 1-bits in VAL. */ 
-int bitcount (unsigned long val);
-
  /* Return 1 if THIS_INSTR might change control flow, 0 otherwise.  */
  int arm_instruction_changes_pc (uint32_t this_instr);
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c 
index d4fed8977e70..8e647e2e2a8e 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -45,6 +45,7 @@
  #include "target-descriptions.h"
  #include "user-regs.h"
  #include "observable.h"
+#include "count-one-bits.h"
#include "arch/arm.h" 
  #include "arch/arm-get-next-pcs.h"
@@ -5798,7 +5799,8 @@ cleanup_block_store_pc (struct gdbarch *gdbarch, struct regcache *regs,
  {
    uint32_t status = displaced_read_reg (regs, dsc, ARM_PS_REGNUM);
    int store_executed = condition_true (dsc->u.block.cond, status);
-  CORE_ADDR pc_stored_at, transferred_regs = bitcount (dsc->u.block.regmask);
+  CORE_ADDR pc_stored_at, transferred_regs
+    = count_one_bits (dsc->u.block.regmask);
    CORE_ADDR stm_insn_addr;
    uint32_t pc_val;
    long offset;
@@ -5850,7 +5852,7 @@ cleanup_block_load_pc (struct gdbarch *gdbarch,
    uint32_t status = displaced_read_reg (regs, dsc, ARM_PS_REGNUM);
    int load_executed = condition_true (dsc->u.block.cond, status);
    unsigned int mask = dsc->u.block.regmask, write_reg = ARM_PC_REGNUM;
-  unsigned int regs_loaded = bitcount (mask);
+  unsigned int regs_loaded = count_one_bits (mask);
    unsigned int num_to_shuffle = regs_loaded, clobbered;
/* The method employed here will fail if the register list is fully populated 
@@ -5982,7 +5984,7 @@ arm_copy_block_xfer (struct gdbarch *gdbarch, uint32_t insn,
  	     contiguous chunk r0...rX before doing the transfer, then shuffling
  	     registers into the correct places in the cleanup routine.  */
  	  unsigned int regmask = insn & 0xffff;
-	  unsigned int num_in_list = bitcount (regmask), new_regmask;
+	  unsigned int num_in_list = count_one_bits (regmask), new_regmask;
  	  unsigned int i;
for (i = 0; i < num_in_list; i++) 
@@ -6084,7 +6086,7 @@ thumb2_copy_block_xfer (struct gdbarch *gdbarch, uint16_t insn1, uint16_t insn2,
        else
  	{
  	  unsigned int regmask = dsc->u.block.regmask;
-	  unsigned int num_in_list = bitcount (regmask), new_regmask;
+	  unsigned int num_in_list = count_one_bits (regmask), new_regmask;
  	  unsigned int i;
for (i = 0; i < num_in_list; i++) 
@@ -7102,7 +7104,7 @@ thumb_copy_pop_pc_16bit (struct gdbarch *gdbarch, uint16_t insn1,
      }
    else
      {
-      unsigned int num_in_list = bitcount (dsc->u.block.regmask);
+      unsigned int num_in_list = count_one_bits (dsc->u.block.regmask);
        unsigned int i;
        unsigned int new_regmask;
diff --git a/gnulib/Makefile.in b/gnulib/Makefile.in 
index 8530f3dcf527..67045edc785c 100644
--- a/gnulib/Makefile.in
+++ b/gnulib/Makefile.in
@@ -1,7 +1,7 @@
  # Makefile.in generated by automake 1.15.1 from Makefile.am.
  # @configure_input@
-# Copyright (C) 1994-2020 Free Software Foundation, Inc. 
+# Copyright (C) 1994-2017 Free Software Foundation, Inc.


The year interval was reverted. Is that a problem?

Otherwise the rest LGTM.
Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]