Re: [PATCH v2] gdbserver/s390: Add support for compiled agent expressions.

Andreas Arnez
2016-03-14T16:19:57Z

On Fri, Mar 04 2016, Marcin Kościelnicki wrote:

> These sequences assume a z900+ CPU, like the rest of fast tracepoint
> support.
>
> gdb/gdbserver/ChangeLog:
>
> * linux-s390-low.c (add_insns): New function.
> (s390_emit_prologue): New function.
> (s390_emit_epilogue): New function.
> (s390_emit_add): New function.
> (s390_emit_sub): New function.
> (s390_emit_mul): New function.
> (s390_emit_lsh): New function.
> (s390_emit_rsh_signed): New function.
> (s390_emit_rsh_unsigned): New function.
> (s390_emit_ext): New function.
> (s390_emit_log_not): New function.
> (s390_emit_bit_and): New function.
> (s390_emit_bit_or): New function.
> (s390_emit_bit_xor): New function.
> (s390_emit_bit_not): New function.
> (s390_emit_equal): New function.
> (s390_emit_less_signed): New function.
> (s390_emit_less_unsigned): New function.
> (s390_emit_ref): New function.
> (s390_emit_if_goto): New function.
> (s390_emit_goto): New function.
> (s390_write_goto_address): New function.
> (s390_emit_litpool): New function.
> (s390_emit_const): New function.
> (s390_emit_call): New function.
> (s390_emit_reg): New function.
> (s390_emit_pop): New function.
> (s390_emit_stack_flush): New function.
> (s390_emit_zero_ext): New function.
> (s390_emit_swap): New function.
> (s390_emit_stack_adjust): New function.
> (s390_emit_set_r2): New function.
> (s390_emit_int_call_1): New function.
> (s390_emit_void_call_2): New function.
> (s390_emit_eq_goto): New function.
> (s390_emit_ne_goto): New function.
> (s390_emit_lt_goto): New function.
> (s390_emit_le_goto): New function.
> (s390_emit_gt_goto): New function.
> (s390_emit_ge_goto): New function.
> (s390x_emit_prologue): New function.
> (s390x_emit_epilogue): New function.
> (s390x_emit_add): New function.
> (s390x_emit_sub): New function.
> (s390x_emit_mul): New function.
> (s390x_emit_lsh): New function.
> (s390x_emit_rsh_signed): New function.
> (s390x_emit_rsh_unsigned): New function.
> (s390x_emit_ext): New function.
> (s390x_emit_log_not): New function.
> (s390x_emit_bit_and): New function.
> (s390x_emit_bit_or): New function.
> (s390x_emit_bit_xor): New function.
> (s390x_emit_bit_not): New function.
> (s390x_emit_equal): New function.
> (s390x_emit_less_signed): New function.
> (s390x_emit_less_unsigned): New function.
> (s390x_emit_ref): New function.
> (s390x_emit_if_goto): New function.
> (s390x_emit_const): New function.
> (s390x_emit_call): New function.
> (s390x_emit_reg): New function.
> (s390x_emit_pop): New function.
> (s390x_emit_stack_flush): New function.
> (s390x_emit_zero_ext): New function.
> (s390x_emit_swap): New function.
> (s390x_emit_stack_adjust): New function.
> (s390x_emit_int_call_1): New function.
> (s390x_emit_void_call_2): New function.
> (s390x_emit_eq_goto): New function.
> (s390x_emit_ne_goto): New function.
> (s390x_emit_lt_goto): New function.
> (s390x_emit_le_goto): New function.
> (s390x_emit_gt_goto): New function.
> (s390x_emit_ge_goto): New function.
> (s390_emit_ops): New function.
> (struct linux_target_ops): Fill in emit_ops hook.
> ---
> Updated version: deduplicated s390_emit_goto with s390x_emit_goto,
> documentation comments, indent fixes.

This is OK.

One minor nit:

> +static void
> +s390_emit_ext (int arg)
> +{
> + unsigned char buf[] = {
> + 0x8d, 0x20, 0x00, 64-arg, /* sldl %r2, <64-arg> */
> + 0x8e, 0x20, 0x00, 64-arg, /* srda %r2, <64-arg> */

The "-" operator should be separated with spaces from its operands. The
same applies to s390_emit_zero_ext, s390x_emit_ext, and
s390x_emit_zero_ext.

Thanks,
Andreas