Re: [PATCH 4/5 v5] Tracepoint for ppc64

["Ulrich Weigand" <uweigand at de dot ibm dot com>]

Wei-cheng Wang wrote:

> All the bugs are fixed, except the ppc_fast_tracepoint_valid_at issue.
> I need some time to think about the issue.

I was thinking about a fix along the following lines: for each breakpoint
type, there is a re_set routine that gets called after some event (e.g.
shared library load) happens that might cause a pending breakpoint to now
become active.  For tracepoints, this is:

static void
tracepoint_re_set (struct breakpoint *b)
{
  breakpoint_re_set_default (b);
}

so it simply performs the default action.  At this point, for fast tracepoints
we really should re-verify that newly activated locations are OK for the
target.  Depending on what's easier, this could either be done directly here,
or in breakpoint_re_set_default or update_breakpoint_locations.

If the target rejects the tracepoint location, we could either throw and
error like in dprintf_re_set (which means execution stops and the user
has to figure out how to continue, e.g. by disabling the tracepoints),
or else the tracepoints could be disabled automatically.

This can certainly be done in a follow-on patch.

> I don't think there is any issue (in GDB) causing the slowness.
> The overhead for slow tracepoint is significantly lower on PPC than on X86 system.
> For example, the time for 1,600,000 iterations
> 
> on gcc10 (x86_64):
>   real:  45,120,000
>   user:     180,011
>   sys :   4,116,257
> 
> on gcc112 (ppc64):
>   real: 149,400,000
>   user:      91,719
>   sys :   2,344,530
> 
> The test case tries to find an iteration count that takes user+sys time
> between 2 and 5 seconds.   If user/sys time overhead is lower, it takes
> longer real-time to finish.

Ah, I see.  What's a bit misleading here is that "user" time isn't really
overhead -- that is actual execution time of the inferior.  The "system"
time is indeed overhead, this is time spent in the kernel while executing
system calls on behalf of the inferior.

Now, we see here that even the sum of those two is much lower than wall-
clock time.  This means the CPU is spending a lot of time neither executing
the inferior code nor executing kernel code on its behalf, but something else.
It would appear that the "something else" here is *gdbserver* (or GDB) ...
this should certainly count as overhead, but isn't captured by the "system"
time above.

If overhead spent in gdbserver or GDB while running slow tracepoints
is *higher* on ppc64 compared to x86 (as your numbers above appear
to indicate), this might explain the timeouts.

I think the way to fix this is to change the test case to not watch
user+system time, but rather watch *wallclock* time when trying to
determine the appropriate iteration count (the limit should then
probably be higher than 2..5 seconds, however).

This can also be done in a follow-on patch.

> gdb/ChangeLog
> 
> 2015-07-13  Wei-cheng Wang  <cole945@gmail.com>
> 
> 	* rs6000-tdep.c (ppc_fast_tracepoint_valid_at,
> 	ppc_gen_return_address): New functions.
> 	(rs6000_gdbarch_init): Hook ppc_fast_tracepoint_valid_at,
> 	and ppc_gen_return_address.
> 
> gdb/gdbserver/ChangeLog
> 
> 2015-07-13  Wei-cheng Wang  <cole945@gmail.com>
> 
> 	* Makefile.in (linux-ppc-ipa.o, powerpc-64l-ipa.o,
> 	powerpc-32l-ipa.o): New rules.
> 	* configure.srv (powerpc*-*-linux*): Add powerpc-64l-ipa.o,
> 	powerpc-32l-ipa.o, and linux-ppc-ipa.o in ipa_obj
> 	* linux-ppc-ipa.c: New file.
> 	* linux-ppc-low.c (gen_ds_form, gen_d_form, gen_xfx_form, gen_x_form,
> 	gen_md_form, gen_i_form, gen_b_form, gen_limm,
> 	gen_atomic_xchg, gen_call, ppc_supports_tracepoints,
> 	ppc_install_fast_tracepoint_jump_pad,
> 	ppc_get_min_fast_tracepoint_insn_len, emit_insns, ppc64_emit_prologue,
> 	ppc64_emit_epilogue, ppc64_emit_add, ppc64_emit_sub, ppc64_emit_mul,
> 	ppc64_emit_lsh, ppc64_emit_rsh_signed, ppc64_emit_rsh_unsigned,
> 	ppc64_emit_ext, ppc64_emit_zero_ext, ppc64_emit_log_not,
> 	ppc64_emit_bit_and, ppc64_emit_bit_or, ppc64_emit_bit_xor,
> 	ppc64_emit_bit_not, ppc64_emit_equal, ppc64_emit_less_signed,
> 	ppc64_emit_less_unsigned, ppc64_emit_ref, ppc64_emit_const,
> 	ppc64_emit_reg, ppc64_emit_pop, ppc64_emit_stack_flush,
> 	ppc64_emit_swap, ppc64_emit_stack_adjust, ppc64_emit_call,
> 	ppc64_emit_int_call_1, ppc64_emit_void_call_2, ppc64_emit_if_goto,
> 	ppc64_emit_goto, ppc64_emit_eq_goto, ppc64_emit_ne_goto,
> 	ppc64_emit_lt_goto, ppc64_emit_le_goto, ppc64_emit_gt_goto,
> 	ppc64_emit_ge_goto, ppc_write_goto_address, ppc_emit_ops,
> 	ppc_supports_range_stepping, ppc_fast_tracepoint_valid_at,
> 	ppc_relocate_instruction): New functions.
> 	(ppc64_emit_ops_vector): New struct for bytecode compilation.
> 	(the_low_target): Add target ops - ppc_supports_tracepoints,
> 	ppc_install_fast_tracepoint_jump_pad, ppc_emit_ops,
> 	ppc_get_min_fast_tracepoint_insn_len, ppc_supports_range_stepping.
> 
> gdb/testsuite/ChangeLog
> 
> 2015-07-13  Wei-cheng Wang  <cole945@gmail.com>
> 
> 	* gdb.trace/backtrace.exp: Set registers for powerpc*-*-*.
> 	* gdb.trace/collection.exp: Ditto.
> 	* gdb.trace/entry-values.exp: Ditto.
> 	* gdb.trace/mi-trace-frame-collected.exp: Ditto.
> 	* gdb.trace/mi-trace-unavailable.exp: Ditto.
> 	* gdb.trace/pending.exp: Ditto.
> 	* gdb.trace/report.exp: Ditto.
> 	* gdb.trace/trace-break.exp: Ditto.
> 	* gdb.trace/while-dyn.exp: Ditto.
> 	* gdb.trace/change-loc.h: set_point for powerpc.
> 	* gdb.trace/ftrace.c: Ditto
> 	* gdb.trace/pendshr1.c: Ditto.
> 	* gdb.trace/pendshr2.c: Ditto.
> 	* gdb.trace/range-stepping.c: Ditto.
> 	* gdb.trace/trace-break.c: Ditto.
> 	* gdb.trace/trace-mt.c: Ditto.
> 	* gdb.trace/ftrace.exp: Enable testing for powerpc*-*-*.

This version is OK now.  Thanks again for working on this!

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com