gdb: LoongArch: Optimize fetch register process

Message ID 20230706034406.17525-1-lihui@loongson.cn
State New
Headers
Series gdb: LoongArch: Optimize fetch register process |

Commit Message

Hui Li July 6, 2023, 3:44 a.m. UTC
  For the current code, when a register is fetched, the entire regset
are fetched via ptrace, but only this register status is updated in
regcache. If another register in this regset to be fetched at this
point, it need to fetch the same regset through ptrace again. This
is obviously unnecessary.

This change is to update the status of entire regset in regcache after
fetching a register via ptrace. Since the status of the entire regset
in regcache is up to date, If another register in this regset to be
fetched at this point. It will be fetched directly from the regcache,
there is no need to fetch the same regset again via ptrace.

This is just an optimization patch for LoongArch. the following are
test results on LoongArch.

before this patch:

make check-gdb -j4

=== gdb Summary ===

 # of expected passes		83662
 # of unexpected failures	1530

after this patch:

make check-gdb -j4

=== gdb Summary ===

 # of expected passes		83662
 # of unexpected failures	1528

Signed-off-by: Hui Li <lihui@loongson.cn>
---
 gdb/loongarch-linux-nat.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
  

Comments

Tom Tromey July 6, 2023, 3:36 p.m. UTC | #1
>>>>> Hui Li <lihui@loongson.cn> writes:

> For the current code, when a register is fetched, the entire regset
> are fetched via ptrace, but only this register status is updated in
> regcache. If another register in this regset to be fetched at this
> point, it need to fetch the same regset through ptrace again. This
> is obviously unnecessary.

> This change is to update the status of entire regset in regcache after
> fetching a register via ptrace. Since the status of the entire regset
> in regcache is up to date, If another register in this regset to be
> fetched at this point. It will be fetched directly from the regcache,
> there is no need to fetch the same regset again via ptrace.

Makes sense to me.  I see other arches do this as well.

Reviewed-By: Tom Tromey <tom@tromey.com>

Tom
  
Tiezhu Yang July 19, 2023, 2:19 p.m. UTC | #2
On 7/6/23 11:44, Hui Li wrote:
> For the current code, when a register is fetched, the entire regset
> are fetched via ptrace, but only this register status is updated in
> regcache. If another register in this regset to be fetched at this
> point, it need to fetch the same regset through ptrace again. This
> is obviously unnecessary.
> 
> This change is to update the status of entire regset in regcache after
> fetching a register via ptrace. Since the status of the entire regset
> in regcache is up to date, If another register in this regset to be
> fetched at this point. It will be fetched directly from the regcache,
> there is no need to fetch the same regset again via ptrace.
> 
> This is just an optimization patch for LoongArch. the following are
> test results on LoongArch.
> 
> before this patch:
> 
> make check-gdb -j4
> 
> === gdb Summary ===
> 
>   # of expected passes		83662
>   # of unexpected failures	1530
> 
> after this patch:
> 
> make check-gdb -j4
> 
> === gdb Summary ===
> 
>   # of expected passes		83662
>   # of unexpected failures	1528
> 
> Signed-off-by: Hui Li <lihui@loongson.cn>
> ---
>   gdb/loongarch-linux-nat.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/gdb/loongarch-linux-nat.c b/gdb/loongarch-linux-nat.c
> index 5eb32b0a52a..40231d5d753 100644
> --- a/gdb/loongarch-linux-nat.c
> +++ b/gdb/loongarch-linux-nat.c
> @@ -65,7 +65,7 @@ fetch_gregs_from_thread (struct regcache *regcache, int regnum, pid_t tid)
>       if (ptrace (PTRACE_GETREGSET, tid, NT_PRSTATUS, (long) &iov) < 0)
>         perror_with_name (_("Couldn't get NT_PRSTATUS registers"));
>       else
> -      loongarch_gregset.supply_regset (nullptr, regcache, regnum,
> +      loongarch_gregset.supply_regset (nullptr, regcache, -1,
>   				       &regset, sizeof (regset));
>     }
>   }
> @@ -116,7 +116,7 @@ fetch_fpregs_from_thread (struct regcache *regcache, int regnum, pid_t tid)
>         if (ptrace (PTRACE_GETREGSET, tid, NT_FPREGSET, (long) &iovec) < 0)
>   	perror_with_name (_("Couldn't get NT_FPREGSET registers"));
>         else
> -	loongarch_fpregset.supply_regset (nullptr, regcache, regnum,
> +	loongarch_fpregset.supply_regset (nullptr, regcache, -1,
>   					  &regset, sizeof (regset));
>       }
>   }

Rename the patch title and update the commit message, pushed.

https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=fc07c81340b8

Thanks,
Tiezhu
  

Patch

diff --git a/gdb/loongarch-linux-nat.c b/gdb/loongarch-linux-nat.c
index 5eb32b0a52a..40231d5d753 100644
--- a/gdb/loongarch-linux-nat.c
+++ b/gdb/loongarch-linux-nat.c
@@ -65,7 +65,7 @@  fetch_gregs_from_thread (struct regcache *regcache, int regnum, pid_t tid)
     if (ptrace (PTRACE_GETREGSET, tid, NT_PRSTATUS, (long) &iov) < 0)
       perror_with_name (_("Couldn't get NT_PRSTATUS registers"));
     else
-      loongarch_gregset.supply_regset (nullptr, regcache, regnum,
+      loongarch_gregset.supply_regset (nullptr, regcache, -1,
 				       &regset, sizeof (regset));
   }
 }
@@ -116,7 +116,7 @@  fetch_fpregs_from_thread (struct regcache *regcache, int regnum, pid_t tid)
       if (ptrace (PTRACE_GETREGSET, tid, NT_FPREGSET, (long) &iovec) < 0)
 	perror_with_name (_("Couldn't get NT_FPREGSET registers"));
       else
-	loongarch_fpregset.supply_regset (nullptr, regcache, regnum,
+	loongarch_fpregset.supply_regset (nullptr, regcache, -1,
 					  &regset, sizeof (regset));
     }
 }