Re: [PATCH v4] gdbserver/s390: Switch on tracepoint support.

[Andreas Arnez <arnez@linux.vnet.ibm.com>]

On Tue, Mar 15 2016, Marcin Kościelnicki wrote:

> Also adds s390 support to gdb.trace testsuite.
>
> gdb/gdbserver/ChangeLog:
>
> 	* linux-s390-low.c (s390_supports_tracepoints): New function.
> 	(struct linux_target_ops): Fill supports_tracepoints hook.
>
> gdb/testsuite/ChangeLog:
>
> 	* gdb.trace/ftrace.exp: Set arg0exp for s390.
> 	* gdb.trace/mi-trace-frame-collected.exp: Expect 4 registers on s390.
> 	* gdb.trace/mi-trace-unavailable.exp: Set pcnum for s390, add gpr0num
> 	variable for GPR 0 instead of assuming it is register 0.
> 	* gdb.trace/trace-common.h: Add s390 fast tracepoint placeholder.
> 	* lib/trace-support.exp: Add s390 registers.
> ---
> This fixes a minor conflict with the powerpc regular tracepoint support
> pushed in the meantime (mi-trace-unavailable.exp now needs gpr0num in
> powerpc branch).

Please also document this new feature in the NEWS file.

[...]

> diff --git a/gdb/testsuite/lib/trace-support.exp b/gdb/testsuite/lib/trace-support.exp
> index 372a595..b307f3f 100644
> --- a/gdb/testsuite/lib/trace-support.exp
> +++ b/gdb/testsuite/lib/trace-support.exp
> @@ -40,6 +40,10 @@ if [is_amd64_regs_target] {
>      set fpreg "r31"
>      set spreg "r1"
>      set pcreg "pc"
> +} elseif { [istarget "s390*-*-*"] } {
> +    set fpreg "r11"
> +    set spreg "r15"
> +    set pcreg "pc"
>  } else {
>      set fpreg "fp"
>      set spreg "sp"

Without having looked into the details I wonder why we can't use the
default, fp, sp, and pc.  Also, I'd slightly prefer initializing these
variables with their default values first and then adjusting them per
target as required.  (This is just a matter of taste.)

Otherwise the patch looks good to me.  But I think a global- or
testsuite-maintainer should approve the changes to the testsuite.

Thanks,
Andreas