[PATCH 0/3] Use code cache in arm and aarch64 prologue analyzer

[PATCH 0/3] Use code cache in arm and aarch64 prologue analyzer

[Yao Qi]

This patch series improves the speed on setting a breakpoint on a
function, like "break foo" on arm and aarch64.  Each backend has
slightly different paths on "debug information exists vs. debug
information doesn't exist".  Patch #1 changes gdb.perf/skip-prologue.exp
so that we can compile the test case with and without debug
information.  Patch #2 and patch #3 improves arm and aarch respectively.

*** BLURB HERE ***

Yao Qi (3):
  Compile gdb.perf/skip-prologue.c with and without debug info
  Use code cache in arm prologue analyzer
  Use code cache in aarch64 prologue analyzer

 gdb/aarch64-tdep.c                       |  2 +-
 gdb/arm-tdep.c                           | 38 ++++++++++++++++----------------
 gdb/testsuite/gdb.perf/skip-prologue.exp |  8 ++++++-
 3 files changed, 27 insertions(+), 21 deletions(-)

-- 
1.9.1

[PATCH 3/3] Use code cache in aarch64 prologue analyzer

[Yao Qi]

This patch change aarch prologue analyzer using code cache, in order
to improve the performance of remote debugging.

gdb.perf/skip-prologue.exp (measured by wall-time) is improved when
the program is compiled without debug information.

			Original	Patched		Original	Patched
			without dbg	without dbg	with dbg	with dbg

/			11.1635239124	9.99472999573	9.65339517593	9.66648793221
-fstack-protector-all	11.2560930252	9.338118	9.63896489143	9.59474396706

gdb:

2016-12-06  Yao Qi  <yao.qi@linaro.org>

	* aarch64-tdep.c (instruction_reader::read): Call
	read_code_unsigned_integer instead of
	read_memory_unsigned_integer.
---
 gdb/aarch64-tdep.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index 590dcf6..0175630 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -213,7 +213,7 @@ class instruction_reader : public abstract_instruction_reader
  public:
   ULONGEST read (CORE_ADDR memaddr, int len, enum bfd_endian byte_order)
   {
-    return read_memory_unsigned_integer (memaddr, len, byte_order);
+    return read_code_unsigned_integer (memaddr, len, byte_order);
   }
 };
 
-- 
1.9.1

Re: [PATCH 3/3] Use code cache in aarch64 prologue analyzer

[Luis Machado]

On 12/06/2016 09:03 AM, Yao Qi wrote:
> This patch change aarch prologue analyzer using code cache, in order
> to improve the performance of remote debugging.
>
> gdb.perf/skip-prologue.exp (measured by wall-time) is improved when
> the program is compiled without debug information.
>
> 			Original	Patched		Original	Patched
> 			without dbg	without dbg	with dbg	with dbg
>
> /			11.1635239124	9.99472999573	9.65339517593	9.66648793221
> -fstack-protector-all	11.2560930252	9.338118	9.63896489143	9.59474396706
>
> gdb:
>
> 2016-12-06  Yao Qi  <yao.qi@linaro.org>
>
> 	* aarch64-tdep.c (instruction_reader::read): Call
> 	read_code_unsigned_integer instead of
> 	read_memory_unsigned_integer.
> ---
>  gdb/aarch64-tdep.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
> index 590dcf6..0175630 100644
> --- a/gdb/aarch64-tdep.c
> +++ b/gdb/aarch64-tdep.c
> @@ -213,7 +213,7 @@ class instruction_reader : public abstract_instruction_reader
>   public:
>    ULONGEST read (CORE_ADDR memaddr, int len, enum bfd_endian byte_order)
>    {
> -    return read_memory_unsigned_integer (memaddr, len, byte_order);
> +    return read_code_unsigned_integer (memaddr, len, byte_order);
>    }
>  };
>
>

The series LGTM except for the nit in 1/3. FTR, we have a similar local 
patch for ARM and also PowerPC to get the performance numbers up a bit. 
It helps with remote debugging as well, since we can read from cache 
instead of fetching it over and over again from the target.

Re: [PATCH 3/3] Use code cache in aarch64 prologue analyzer

[Yao Qi]

On 16-12-07 12:26:38, Luis Machado wrote:
> >
> >
> 
> The series LGTM except for the nit in 1/3. FTR, we have a similar
> local patch for ARM and also PowerPC to get the performance numbers
> up a bit. It helps with remote debugging as well, since we can read
> from cache instead of fetching it over and over again from the
> target.

Yes, I still have some patches for ARM to use cache in arm-tdep.c.
I plan to send them out after I measured the performance improvement,
but I need to fix PR 20939 before 7.12.1 (released on Jan 2017), so
I won't post my ARM patches recently.  If you already get your local
patches in shape, free feel to post them.

-- 
Yao (齐尧)

[PATCH 2/3] Use code cache in arm prologue analyzer

[Yao Qi]

This patch change arm prologue analyzer using code cache, in order
to improve the performance of remote debugging.

gdb.perf/skip-prologue.exp (measured by wall-time) is improved a lot,

			Original	Patched		Original	Patched
			without dbg	without dbg	with dbg	with dbg

-marm			14.166741848	9.32852292061  	11.4908499718  	9.16302204132
-marm   		14.6705040932  	9.34849786758  	18.2788009644  	9.14823913574
\-fstack-protector-all
-mthumb			34.4391930103	10.6062178612 	13.7886838913	10.3094120026
-mthumb
\-fstack-protector-all	34.9310460091	10.6413481236	25.3875930309	10.6294929981

gdb:

2016-12-06  Yao Qi  <yao.qi@linaro.org>

	* arm-tdep.c (skip_prologue_function): Call
	read_code_unsigned_integer instead of
	read_memory_unsigned_integer.
	(thumb_analyze_prologue): Likewise.
	(arm_analyze_load_stack_chk_guard): Likewise.
	(arm_skip_stack_protector): Likewise.
	(arm_analyze_prologue):Likewise.
	(extend_buffer_earlier): Call target_read_code instead
	of target_read_memory.
	(arm_adjust_breakpoint_address): Likewise.
---
 gdb/arm-tdep.c | 38 +++++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index fd5d0de..97dea89 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -570,9 +570,9 @@ skip_prologue_function (struct gdbarch *gdbarch, CORE_ADDR pc, int is_thumb)
 	 implementation (this is hand-written ARM assembler in glibc).  */
 
       if (!is_thumb
-	  && read_memory_unsigned_integer (pc, 4, byte_order_for_code)
+	  && read_code_unsigned_integer (pc, 4, byte_order_for_code)
 	     == 0xe3e00a0f /* mov r0, #0xffff0fff */
-	  && read_memory_unsigned_integer (pc + 4, 4, byte_order_for_code)
+	  && read_code_unsigned_integer (pc + 4, 4, byte_order_for_code)
 	     == 0xe240f01f) /* sub pc, r0, #31 */
 	return 1;
     }
@@ -659,7 +659,7 @@ thumb_analyze_prologue (struct gdbarch *gdbarch,
     {
       unsigned short insn;
 
-      insn = read_memory_unsigned_integer (start, 2, byte_order_for_code);
+      insn = read_code_unsigned_integer (start, 2, byte_order_for_code);
 
       if ((insn & 0xfe00) == 0xb400)		/* push { rlist } */
 	{
@@ -790,8 +790,8 @@ thumb_analyze_prologue (struct gdbarch *gdbarch,
 	{
 	  unsigned short inst2;
 
-	  inst2 = read_memory_unsigned_integer (start + 2, 2,
-						byte_order_for_code);
+	  inst2 = read_code_unsigned_integer (start + 2, 2,
+					      byte_order_for_code);
 
 	  if ((insn & 0xf800) == 0xf000 && (inst2 & 0xe800) == 0xe800)
 	    {
@@ -1134,7 +1134,7 @@ arm_analyze_load_stack_chk_guard(CORE_ADDR pc, struct gdbarch *gdbarch,
   if (is_thumb)
     {
       unsigned short insn1
-	= read_memory_unsigned_integer (pc, 2, byte_order_for_code);
+	= read_code_unsigned_integer (pc, 2, byte_order_for_code);
 
       if ((insn1 & 0xf800) == 0x4800) /* ldr Rd, #immed */
 	{
@@ -1147,14 +1147,14 @@ arm_analyze_load_stack_chk_guard(CORE_ADDR pc, struct gdbarch *gdbarch,
       else if ((insn1 & 0xfbf0) == 0xf240) /* movw Rd, #const */
 	{
 	  unsigned short insn2
-	    = read_memory_unsigned_integer (pc + 2, 2, byte_order_for_code);
+	    = read_code_unsigned_integer (pc + 2, 2, byte_order_for_code);
 
 	  low = EXTRACT_MOVW_MOVT_IMM_T (insn1, insn2);
 
 	  insn1
-	    = read_memory_unsigned_integer (pc + 4, 2, byte_order_for_code);
+	    = read_code_unsigned_integer (pc + 4, 2, byte_order_for_code);
 	  insn2
-	    = read_memory_unsigned_integer (pc + 6, 2, byte_order_for_code);
+	    = read_code_unsigned_integer (pc + 6, 2, byte_order_for_code);
 
 	  /* movt Rd, #const */
 	  if ((insn1 & 0xfbc0) == 0xf2c0)
@@ -1169,7 +1169,7 @@ arm_analyze_load_stack_chk_guard(CORE_ADDR pc, struct gdbarch *gdbarch,
   else
     {
       unsigned int insn
-	= read_memory_unsigned_integer (pc, 4, byte_order_for_code);
+	= read_code_unsigned_integer (pc, 4, byte_order_for_code);
 
       if ((insn & 0x0e5f0000) == 0x041f0000) /* ldr Rd, [PC, #immed] */
 	{
@@ -1185,7 +1185,7 @@ arm_analyze_load_stack_chk_guard(CORE_ADDR pc, struct gdbarch *gdbarch,
 	  low = EXTRACT_MOVW_MOVT_IMM_A (insn);
 
 	  insn
-	    = read_memory_unsigned_integer (pc + 4, 4, byte_order_for_code);
+	    = read_code_unsigned_integer (pc + 4, 4, byte_order_for_code);
 
 	  if ((insn & 0x0ff00000) == 0x03400000) /* movt Rd, #const */
 	    {
@@ -1257,7 +1257,7 @@ arm_skip_stack_protector(CORE_ADDR pc, struct gdbarch *gdbarch)
     {
       unsigned int destreg;
       unsigned short insn
-	= read_memory_unsigned_integer (pc + offset, 2, byte_order_for_code);
+	= read_code_unsigned_integer (pc + offset, 2, byte_order_for_code);
 
       /* Step 2: ldr Rd, [Rn, #immed], encoding T1.  */
       if ((insn & 0xf800) != 0x6800)
@@ -1266,8 +1266,8 @@ arm_skip_stack_protector(CORE_ADDR pc, struct gdbarch *gdbarch)
 	return pc;
       destreg = bits (insn, 0, 2);
 
-      insn = read_memory_unsigned_integer (pc + offset + 2, 2,
-					   byte_order_for_code);
+      insn = read_code_unsigned_integer (pc + offset + 2, 2,
+					 byte_order_for_code);
       /* Step 3: str Rd, [Rn, #immed], encoding T1.  */
       if ((insn & 0xf800) != 0x6000)
 	return pc;
@@ -1278,7 +1278,7 @@ arm_skip_stack_protector(CORE_ADDR pc, struct gdbarch *gdbarch)
     {
       unsigned int destreg;
       unsigned int insn
-	= read_memory_unsigned_integer (pc + offset, 4, byte_order_for_code);
+	= read_code_unsigned_integer (pc + offset, 4, byte_order_for_code);
 
       /* Step 2: ldr Rd, [Rn, #immed], encoding A1.  */
       if ((insn & 0x0e500000) != 0x04100000)
@@ -1287,7 +1287,7 @@ arm_skip_stack_protector(CORE_ADDR pc, struct gdbarch *gdbarch)
 	return pc;
       destreg = bits (insn, 12, 15);
       /* Step 3: str Rd, [Rn, #immed], encoding A1.  */
-      insn = read_memory_unsigned_integer (pc + offset + 4,
+      insn = read_code_unsigned_integer (pc + offset + 4,
 					   4, byte_order_for_code);
       if ((insn & 0x0e500000) != 0x04000000)
 	return pc;
@@ -1511,7 +1511,7 @@ arm_analyze_prologue (struct gdbarch *gdbarch,
        current_pc += 4)
     {
       unsigned int insn
-	= read_memory_unsigned_integer (current_pc, 4, byte_order_for_code);
+	= read_code_unsigned_integer (current_pc, 4, byte_order_for_code);
 
       if (insn == 0xe1a0c00d)		/* mov ip, sp */
 	{
@@ -4250,7 +4250,7 @@ extend_buffer_earlier (gdb_byte *buf, CORE_ADDR endaddr,
   new_buf = (gdb_byte *) xmalloc (new_len);
   memcpy (new_buf + bytes_to_read, buf, old_len);
   xfree (buf);
-  if (target_read_memory (endaddr - new_len, new_buf, bytes_to_read) != 0)
+  if (target_read_code (endaddr - new_len, new_buf, bytes_to_read) != 0)
     {
       xfree (new_buf);
       return NULL;
@@ -4314,7 +4314,7 @@ arm_adjust_breakpoint_address (struct gdbarch *gdbarch, CORE_ADDR bpaddr)
     return bpaddr;
 
   buf = (gdb_byte *) xmalloc (buf_len);
-  if (target_read_memory (bpaddr - buf_len, buf, buf_len) != 0)
+  if (target_read_code (bpaddr - buf_len, buf, buf_len) != 0)
     return bpaddr;
   any = 0;
   for (i = 0; i < buf_len; i += 2)
-- 
1.9.1

[PATCH 1/3] Compile gdb.perf/skip-prologue.c with and without debug info

[Yao Qi]

gdb.perf/skip-prologue.exp is intended to measure the performance of
skipping prologue with prologue analysis by setting breakpoints.
However, if program is compiled with debug info, GDB is smart to
skip prologue by line table from debug info, so prologue analysis
is not exercised at all.

This patch adds a parameter COMPILE to specify compiling with
debug information, otherwise, it is compiled without debug
information.

gdb/testsuite:

2016-12-06  Yao Qi  <yao.qi@linaro.org>

	* gdb.perf/skip-prologue.exp: Add parameter COMPILE.
---
 gdb/testsuite/gdb.perf/skip-prologue.exp | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/gdb/testsuite/gdb.perf/skip-prologue.exp b/gdb/testsuite/gdb.perf/skip-prologue.exp
index 2c4aad4..a24fd73 100644
--- a/gdb/testsuite/gdb.perf/skip-prologue.exp
+++ b/gdb/testsuite/gdb.perf/skip-prologue.exp
@@ -37,7 +37,13 @@ if ![info exists SKIP_PROLOGUE_COUNT] {
 PerfTest::assemble {
     global srcdir subdir srcfile binfile
 
-    if { [gdb_compile "$srcdir/$subdir/$srcfile" ${binfile} executable {debug}] != "" } {
+    if [info exists COMPILE] {
+	set opts {debug}
+    } else {
+	set opts {nodebug}
+    }
+
+    if { [gdb_compile "$srcdir/$subdir/$srcfile" ${binfile} executable $opts] != "" } {
 	return -1
     }
 
-- 
1.9.1

Re: [PATCH 1/3] Compile gdb.perf/skip-prologue.c with and without debug info

[Luis Machado]

On 12/06/2016 09:03 AM, Yao Qi wrote:
> gdb.perf/skip-prologue.exp is intended to measure the performance of
> skipping prologue with prologue analysis by setting breakpoints.
> However, if program is compiled with debug info, GDB is smart to
> skip prologue by line table from debug info, so prologue analysis
> is not exercised at all.
>
> This patch adds a parameter COMPILE to specify compiling with
> debug information, otherwise, it is compiled without debug
> information.
>
> gdb/testsuite:
>
> 2016-12-06  Yao Qi  <yao.qi@linaro.org>
>
> 	* gdb.perf/skip-prologue.exp: Add parameter COMPILE.
> ---
>  gdb/testsuite/gdb.perf/skip-prologue.exp | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/gdb/testsuite/gdb.perf/skip-prologue.exp b/gdb/testsuite/gdb.perf/skip-prologue.exp
> index 2c4aad4..a24fd73 100644
> --- a/gdb/testsuite/gdb.perf/skip-prologue.exp
> +++ b/gdb/testsuite/gdb.perf/skip-prologue.exp
> @@ -37,7 +37,13 @@ if ![info exists SKIP_PROLOGUE_COUNT] {
>  PerfTest::assemble {
>      global srcdir subdir srcfile binfile
>
> -    if { [gdb_compile "$srcdir/$subdir/$srcfile" ${binfile} executable {debug}] != "" } {
> +    if [info exists COMPILE] {
> +	set opts {debug}
> +    } else {
> +	set opts {nodebug}
> +    }
> +
> +    if { [gdb_compile "$srcdir/$subdir/$srcfile" ${binfile} executable $opts] != "" } {


Maybe add an untested call here saying "failed to compile"? Just a nit.

Re: [PATCH 1/3] Compile gdb.perf/skip-prologue.c with and without debug info

[Yao Qi]

On 16-12-07 12:24:25, Luis Machado wrote:
> >+
> >+    if { [gdb_compile "$srcdir/$subdir/$srcfile" ${binfile} executable $opts] != "" } {
> 
> 
> Maybe add an untested call here saying "failed to compile"? Just a nit.

Sure, fixed in the patch below.

-- 
Yao (齐尧)

From 364ca42e71dfdce3f34e97ef028f7e3d5b5d880e Mon Sep 17 00:00:00 2001
From: Yao Qi <yao.qi@linaro.org>
Date: Fri, 9 Dec 2016 09:51:20 +0000
Subject: [PATCH] Compile gdb.perf/skip-prologue.c with and without debug info

gdb.perf/skip-prologue.exp is intended to measure the performance of
skipping prologue with prologue analysis by setting breakpoints.
However, if program is compiled with debug info, GDB is smart to
skip prologue by line table from debug info, so prologue analysis
is not exercised at all.

This patch adds a parameter COMPILE to specify compiling with
debug information, otherwise, it is compiled without debug
information.

gdb/testsuite:

2016-12-09  Yao Qi  <yao.qi@linaro.org>

	* gdb.perf/skip-prologue.exp: Add parameter COMPILE.

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index e63f2b7..d905df3 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,7 @@
+2016-12-09  Yao Qi  <yao.qi@linaro.org>
+
+	* gdb.perf/skip-prologue.exp: Add parameter COMPILE.
+
 2016-12-02  Luis Machado  <lgustavo@codesourcery.com>
 
 	* gdb.base/maint.exp: Use gdb_test instead of gdb_test_multiple when
diff --git a/gdb/testsuite/gdb.perf/skip-prologue.exp b/gdb/testsuite/gdb.perf/skip-prologue.exp
index 2c4aad4..f31f697 100644
--- a/gdb/testsuite/gdb.perf/skip-prologue.exp
+++ b/gdb/testsuite/gdb.perf/skip-prologue.exp
@@ -37,7 +37,14 @@ if ![info exists SKIP_PROLOGUE_COUNT] {
 PerfTest::assemble {
     global srcdir subdir srcfile binfile
 
-    if { [gdb_compile "$srcdir/$subdir/$srcfile" ${binfile} executable {debug}] != "" } {
+    if [info exists COMPILE] {
+	set opts {debug}
+    } else {
+	set opts {nodebug}
+    }
+
+    if { [gdb_compile "$srcdir/$subdir/$srcfile" ${binfile} executable $opts] != "" } {
+	untested "failed to compile"
 	return -1
     }
 

Re: [PATCH 0/3] Use code cache in arm and aarch64 prologue analyzer

[Yao Qi]

On 16-12-06 15:03:46, Yao Qi wrote:
> This patch series improves the speed on setting a breakpoint on a
> function, like "break foo" on arm and aarch64.  Each backend has
> slightly different paths on "debug information exists vs. debug
> information doesn't exist".  Patch #1 changes gdb.perf/skip-prologue.exp
> so that we can compile the test case with and without debug
> information.  Patch #2 and patch #3 improves arm and aarch respectively.
> 

I've pushed them in.

-- 
Yao (齐尧)