[PATCH] Speedup lnp_state_machine::handle_special_opcode

[PATCH] Speedup lnp_state_machine::handle_special_opcode

[Richard Biener]

I see for some program at gdb startup

Samples: 102K of event 'cycles:pu', Event count (approx.): 91710925103          
Overhead  Command     Shared Object        Symbol                               
  15.21%  gdb         gdb                  [.] 
lnp_state_machine::handle_special

where the divisions are the places we stall.  The following 
micro-optimizes things but it smells like m_line_header->line_range
is constant, likewise probably m_line_header->maximum_ops_per_instruction
so eventually the divisions could be avoided completely with some
lookup table.

Well.  Micro-optimizing with the patch below improves things
(don't expect [load] CSE over the gdbarch_adjust_dwarf2_line call).

Care to pick up this one-time patch without myself becoming familiar
with gdb testing & patch submission?

Thanks,
Richard.

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 7edbd9d7df..e74383e01d 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -19812,16 +19812,16 @@ void
 lnp_state_machine::handle_special_opcode (unsigned char op_code)
 {
   unsigned char adj_opcode = op_code - m_line_header->opcode_base;
-  CORE_ADDR addr_adj = (((m_op_index
-			  + (adj_opcode / m_line_header->line_range))
+  unsigned char adj_opcode_d = adj_opcode / m_line_header->line_range;
+  unsigned char adj_opcode_r = adj_opcode % m_line_header->line_range;
+  CORE_ADDR addr_adj = (((m_op_index + adj_opcode_d)
 			 / m_line_header->maximum_ops_per_instruction)
 			* m_line_header->minimum_instruction_length);
   m_address += gdbarch_adjust_dwarf2_line (m_gdbarch, addr_adj, true);
-  m_op_index = ((m_op_index + (adj_opcode / m_line_header->line_range))
+  m_op_index = ((m_op_index + adj_opcode_d)
 		% m_line_header->maximum_ops_per_instruction);
 
-  int line_delta = (m_line_header->line_base
-		    + (adj_opcode % m_line_header->line_range));
+  int line_delta = m_line_header->line_base + adj_opcode_r;
   advance_line (line_delta);
   record_line (false);
   m_discriminator = 0;

Re: [PATCH] Speedup lnp_state_machine::handle_special_opcode

[Tom de Vries]

[-- Attachment #1: Type: text/plain, Size: 243 bytes --]

On 13-02-2020 14:03, Richard Biener wrote:
> Care to pick up this one-time patch without myself becoming familiar
> with gdb testing & patch submission?

Of course.  Added ChangeLog entry, build and reg-tested on x86_64-linux.

Thanks,
- Tom


[-- Attachment #2: 0001-gdb-Speedup-lnp_state_machine-handle_special_opcode.patch --]
[-- Type: text/x-patch, Size: 2184 bytes --]

[gdb] Speedup lnp_state_machine::handle_special_opcode

I see for some program at gdb startup:
...
Samples: 102K of event 'cycles:pu', Event count (approx.): 91710925103
Overhead  Command     Shared Object        Symbol
  15.21%  gdb         gdb                  [.]
lnp_state_machine::handle_special
...
where the divisions are the places we stall.  The following
micro-optimizes things but it smells like m_line_header->line_range
is constant, likewise probably m_line_header->maximum_ops_per_instruction
so eventually the divisions could be avoided completely with some
lookup table.

Well.  Micro-optimizing with this patch improves things
(don't expect [load] CSE over the gdbarch_adjust_dwarf2_line call).

Build and reg-tested on x86_64-linux.

gdb/ChangeLog:

2020-02-13  Tom de Vries  <tdevries@suse.de>

	* dwarf2/read.c (lnp_state_machine::handle_special_opcode): Apply CSE
	on expression with division operators.

---
 gdb/dwarf2/read.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 7edbd9d7df..e74383e01d 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -19812,16 +19812,16 @@ void
 lnp_state_machine::handle_special_opcode (unsigned char op_code)
 {
   unsigned char adj_opcode = op_code - m_line_header->opcode_base;
-  CORE_ADDR addr_adj = (((m_op_index
-			  + (adj_opcode / m_line_header->line_range))
+  unsigned char adj_opcode_d = adj_opcode / m_line_header->line_range;
+  unsigned char adj_opcode_r = adj_opcode % m_line_header->line_range;
+  CORE_ADDR addr_adj = (((m_op_index + adj_opcode_d)
 			 / m_line_header->maximum_ops_per_instruction)
 			* m_line_header->minimum_instruction_length);
   m_address += gdbarch_adjust_dwarf2_line (m_gdbarch, addr_adj, true);
-  m_op_index = ((m_op_index + (adj_opcode / m_line_header->line_range))
+  m_op_index = ((m_op_index + adj_opcode_d)
 		% m_line_header->maximum_ops_per_instruction);
 
-  int line_delta = (m_line_header->line_base
-		    + (adj_opcode % m_line_header->line_range));
+  int line_delta = m_line_header->line_base + adj_opcode_r;
   advance_line (line_delta);
   record_line (false);
   m_discriminator = 0;

Re: [PATCH] Speedup lnp_state_machine::handle_special_opcode

[Pedro Alves]

Arguably the the compiler should do this for us.  *cough*  :-)

On 2/13/20 2:23 PM, Tom de Vries wrote:
> On 13-02-2020 14:03, Richard Biener wrote:
>> Care to pick up this one-time patch without myself becoming familiar
>> with gdb testing & patch submission?
> 
> Of course.  Added ChangeLog entry, build and reg-tested on x86_64-linux.

The ChangeLog entry (as well a git author) should reflect the patch's
authorship.

Otherwise OK.

Thanks,
Pedro Alves

Re: [PATCH] Speedup lnp_state_machine::handle_special_opcode

[Tom de Vries]

On 13-02-2020 19:26, Pedro Alves wrote:
> Arguably the the compiler should do this for us.  *cough*  :-)
> 
> On 2/13/20 2:23 PM, Tom de Vries wrote:
>> On 13-02-2020 14:03, Richard Biener wrote:
>>> Care to pick up this one-time patch without myself becoming familiar
>>> with gdb testing & patch submission?
>>
>> Of course.  Added ChangeLog entry, build and reg-tested on x86_64-linux.
> 
> The ChangeLog entry (as well a git author) should reflect the patch's
> authorship.
> 

Committed with those change @
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commit;h=258bf0ee3748d4354e13daf00f02266cafa96389
.

Thanks,
- Tom