Re: RFC: move agent opcodes to common file

[Hui Zhu <teawater@gmail.com>]

Hi Tom,

I think this patch is very good.

And I have a question is if the common/ax.def updated, will the
ax-general.c, gdbserver/tracepoint.c and file that use the ax.h
rebuild?

I want try this patch with myself but looks your add the new op
aop_pick = 0x31, aop_rot = 0x32.

Thanks,
Hui

On Thu, Feb 17, 2011 at 06:03, Tom Tromey <tromey@redhat.com> wrote:
> I'd appreciate comments on this.
>
> When I added agent expression opcodes, I had to make boilerplate
> modifications in four places.  This patch centralizes the agent
> expression opcode definitions and changes all the users to use the new
> file.
>
> This came after my pick+roll patch, so it won't apply directly to HEAD.
>
> Tom
>
> 2011-02-16  Tom Tromey  <tromey@redhat.com>
>
>        * common/ax.def: New file.
>        * ax.h (enum agent_op): Use ax.def.
>        * ax-general.c (aop_map): Use ax.def.
>
> 2011-02-16  Tom Tromey  <tromey@redhat.com>
>
>        * tracepoint.c (enum gdb_agent_op): Use ax.def.
>        (gdb_agent_op_names): Likewise.
>
> diff --git a/gdb/ax-general.c b/gdb/ax-general.c
> index 06ff958..edab4dc 100644
> --- a/gdb/ax-general.c
> +++ b/gdb/ax-general.c
> @@ -338,57 +338,11 @@ ax_tsv (struct agent_expr *x, enum agent_op op, int num)
>
>  struct aop_map aop_map[] =
>  {
> -  {0, 0, 0, 0, 0},
> -  {"float", 0, 0, 0, 0},       /* 0x01 */
> -  {"add", 0, 0, 2, 1},         /* 0x02 */
> -  {"sub", 0, 0, 2, 1},         /* 0x03 */
> -  {"mul", 0, 0, 2, 1},         /* 0x04 */
> -  {"div_signed", 0, 0, 2, 1},  /* 0x05 */
> -  {"div_unsigned", 0, 0, 2, 1},        /* 0x06 */
> -  {"rem_signed", 0, 0, 2, 1},  /* 0x07 */
> -  {"rem_unsigned", 0, 0, 2, 1},        /* 0x08 */
> -  {"lsh", 0, 0, 2, 1},         /* 0x09 */
> -  {"rsh_signed", 0, 0, 2, 1},  /* 0x0a */
> -  {"rsh_unsigned", 0, 0, 2, 1},        /* 0x0b */
> -  {"trace", 0, 0, 2, 0},       /* 0x0c */
> -  {"trace_quick", 1, 0, 1, 1}, /* 0x0d */
> -  {"log_not", 0, 0, 1, 1},     /* 0x0e */
> -  {"bit_and", 0, 0, 2, 1},     /* 0x0f */
> -  {"bit_or", 0, 0, 2, 1},      /* 0x10 */
> -  {"bit_xor", 0, 0, 2, 1},     /* 0x11 */
> -  {"bit_not", 0, 0, 1, 1},     /* 0x12 */
> -  {"equal", 0, 0, 2, 1},       /* 0x13 */
> -  {"less_signed", 0, 0, 2, 1}, /* 0x14 */
> -  {"less_unsigned", 0, 0, 2, 1},       /* 0x15 */
> -  {"ext", 1, 0, 1, 1},         /* 0x16 */
> -  {"ref8", 0, 8, 1, 1},                /* 0x17 */
> -  {"ref16", 0, 16, 1, 1},      /* 0x18 */
> -  {"ref32", 0, 32, 1, 1},      /* 0x19 */
> -  {"ref64", 0, 64, 1, 1},      /* 0x1a */
> -  {"ref_float", 0, 0, 1, 1},   /* 0x1b */
> -  {"ref_double", 0, 0, 1, 1},  /* 0x1c */
> -  {"ref_long_double", 0, 0, 1, 1},     /* 0x1d */
> -  {"l_to_d", 0, 0, 1, 1},      /* 0x1e */
> -  {"d_to_l", 0, 0, 1, 1},      /* 0x1f */
> -  {"if_goto", 2, 0, 1, 0},     /* 0x20 */
> -  {"goto", 2, 0, 0, 0},                /* 0x21 */
> -  {"const8", 1, 8, 0, 1},      /* 0x22 */
> -  {"const16", 2, 16, 0, 1},    /* 0x23 */
> -  {"const32", 4, 32, 0, 1},    /* 0x24 */
> -  {"const64", 8, 64, 0, 1},    /* 0x25 */
> -  {"reg", 2, 0, 0, 1},         /* 0x26 */
> -  {"end", 0, 0, 0, 0},         /* 0x27 */
> -  {"dup", 0, 0, 1, 2},         /* 0x28 */
> -  {"pop", 0, 0, 1, 0},         /* 0x29 */
> -  {"zero_ext", 1, 0, 1, 1},    /* 0x2a */
> -  {"swap", 0, 0, 2, 2},                /* 0x2b */
> -  {"getv", 2, 0, 0, 1},                /* 0x2c */
> -  {"setv", 2, 0, 0, 1},                /* 0x2d */
> -  {"tracev", 2, 0, 0, 1},      /* 0x2e */
> -  {0, 0, 0, 0, 0},             /* 0x2f */
> -  {"trace16", 2, 0, 1, 1},     /* 0x30 */
> -  {"pick", 1, 0, 0, 1},                /* 0x31 */
> -  {"roll", 0, 0, 3, 3},                /* 0x32 */
> +  {0, 0, 0, 0, 0}
> +#define DEFOP(NAME, SIZE, DATA_SIZE, CONSUMED, PRODUCED, VALUE) \
> +  , { # NAME, SIZE, DATA_SIZE, CONSUMED, PRODUCED }
> +#include "ax.def"
> +#undef DEFOP
>  };
>
>
> diff --git a/gdb/ax.h b/gdb/ax.h
> index e34aa07..f132af8 100644
> --- a/gdb/ax.h
> +++ b/gdb/ax.h
> @@ -145,67 +145,14 @@ struct agent_expr
>     unsigned char *reg_mask;
>   };
>
> -/* The actual values of the various bytecode operations.
> -
> -   Other independent implementations of the agent bytecode engine will
> -   rely on the exact values of these enums, and may not be recompiled
> -   when we change this table.  The numeric values should remain fixed
> -   whenever possible.  Thus, we assign them values explicitly here (to
> -   allow gaps to form safely), and the disassembly table in
> -   agentexpr.h behaves like an opcode map.  If you want to see them
> -   grouped logically, see doc/agentexpr.texi.  */
> +/* The actual values of the various bytecode operations.  */
>
>  enum agent_op
>   {
> -    aop_float = 0x01,
> -    aop_add = 0x02,
> -    aop_sub = 0x03,
> -    aop_mul = 0x04,
> -    aop_div_signed = 0x05,
> -    aop_div_unsigned = 0x06,
> -    aop_rem_signed = 0x07,
> -    aop_rem_unsigned = 0x08,
> -    aop_lsh = 0x09,
> -    aop_rsh_signed = 0x0a,
> -    aop_rsh_unsigned = 0x0b,
> -    aop_trace = 0x0c,
> -    aop_trace_quick = 0x0d,
> -    aop_log_not = 0x0e,
> -    aop_bit_and = 0x0f,
> -    aop_bit_or = 0x10,
> -    aop_bit_xor = 0x11,
> -    aop_bit_not = 0x12,
> -    aop_equal = 0x13,
> -    aop_less_signed = 0x14,
> -    aop_less_unsigned = 0x15,
> -    aop_ext = 0x16,
> -    aop_ref8 = 0x17,
> -    aop_ref16 = 0x18,
> -    aop_ref32 = 0x19,
> -    aop_ref64 = 0x1a,
> -    aop_ref_float = 0x1b,
> -    aop_ref_double = 0x1c,
> -    aop_ref_long_double = 0x1d,
> -    aop_l_to_d = 0x1e,
> -    aop_d_to_l = 0x1f,
> -    aop_if_goto = 0x20,
> -    aop_goto = 0x21,
> -    aop_const8 = 0x22,
> -    aop_const16 = 0x23,
> -    aop_const32 = 0x24,
> -    aop_const64 = 0x25,
> -    aop_reg = 0x26,
> -    aop_end = 0x27,
> -    aop_dup = 0x28,
> -    aop_pop = 0x29,
> -    aop_zero_ext = 0x2a,
> -    aop_swap = 0x2b,
> -    aop_getv = 0x2c,
> -    aop_setv = 0x2d,
> -    aop_tracev = 0x2e,
> -    aop_trace16 = 0x30,
> -    aop_pick = 0x31,
> -    aop_rot = 0x32,
> +#define DEFOP(NAME, SIZE, DATA_SIZE, CONSUMED, PRODUCED, VALUE)  \
> +    aop_ ## NAME = VALUE,
> +#include "ax.def"
> +#undef DEFOP
>     aop_last
>   };
>
> diff --git a/gdb/common/ax.def b/gdb/common/ax.def
> new file mode 100644
> index 0000000..52b5ab2
> --- /dev/null
> +++ b/gdb/common/ax.def
> @@ -0,0 +1,95 @@
> +/* Definition of agent opcode values.   -*- c -*-
> +   Copyright (C) 1998, 1999, 2000, 2007, 2008, 2009, 2010, 2011
> +   Free Software Foundation, Inc.
> +
> +   This file is part of GDB.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +/* The actual values of the various bytecode operations.
> +
> +   Other independent implementations of the agent bytecode engine will
> +   rely on the exact values of these enums, and may not be recompiled
> +   when we change this table.  The numeric values should remain fixed
> +   whenever possible.  Thus, we assign them values explicitly here (to
> +   allow gaps to form safely), and the disassembly table in
> +   agentexpr.h behaves like an opcode map.  If you want to see them
> +   grouped logically, see doc/agentexpr.texi.
> +
> +   Each line is of the form:
> +
> +   DEFOP (name, size, data_size, consumed, produced, opcode)
> +
> +   NAME is the name of the operation.
> +   SIZE is the number of argument bytes that the operation takes from
> +   the bytecode stream.
> +   DATA_SIZE is the size of data operated on, in bits, for operations
> +   that care (ref and const).  It is zero otherwise.
> +   CONSUMED is the number of stack elements consumed.
> +   PRODUCED is the number of stack elements produced.
> +   OPCODE is the operation's encoding.  */
> +
> +DEFOP (float, 0, 0, 0, 0, 0x01)
> +DEFOP (add, 0, 0, 2, 1, 0x02)
> +DEFOP (sub, 0, 0, 2, 1, 0x03)
> +DEFOP (mul, 0, 0, 2, 1, 0x04)
> +DEFOP (div_signed, 0, 0, 2, 1, 0x05)
> +DEFOP (div_unsigned, 0, 0, 2, 1, 0x06)
> +DEFOP (rem_signed, 0, 0, 2, 1, 0x07)
> +DEFOP (rem_unsigned, 0, 0, 2, 1, 0x08)
> +DEFOP (lsh, 0, 0, 2, 1, 0x09)
> +DEFOP (rsh_signed, 0, 0, 2, 1, 0x0a)
> +DEFOP (rsh_unsigned, 0, 0, 2, 1, 0x0b)
> +DEFOP (trace, 0, 0, 2, 0, 0x0c)
> +DEFOP (trace_quick, 1, 0, 1, 1, 0x0d)
> +DEFOP (log_not, 0, 0, 1, 1, 0x0e)
> +DEFOP (bit_and, 0, 0, 2, 1, 0x0f)
> +DEFOP (bit_or, 0, 0, 2, 1, 0x10)
> +DEFOP (bit_xor, 0, 0, 2, 1, 0x11)
> +DEFOP (bit_not, 0, 0, 1, 1, 0x12)
> +DEFOP (equal, 0, 0, 2, 1, 0x13)
> +DEFOP (less_signed, 0, 0, 2, 1, 0x14)
> +DEFOP (less_unsigned, 0, 0, 2, 1, 0x15)
> +DEFOP (ext, 1, 0, 1, 1, 0x16)
> +DEFOP (ref8, 0, 8, 1, 1, 0x17)
> +DEFOP (ref16, 0, 16, 1, 1, 0x18)
> +DEFOP (ref32, 0, 32, 1, 1, 0x19)
> +DEFOP (ref64, 0, 64, 1, 1, 0x1a)
> +DEFOP (ref_float, 0, 0, 1, 1, 0x1b)
> +DEFOP (ref_double, 0, 0, 1, 1, 0x1c)
> +DEFOP (ref_long_double, 0, 0, 1, 1, 0x1d)
> +DEFOP (l_to_d, 0, 0, 1, 1, 0x1e)
> +DEFOP (d_to_l, 0, 0, 1, 1, 0x1f)
> +DEFOP (if_goto, 2, 0, 1, 0, 0x20)
> +DEFOP (goto, 2, 0, 0, 0, 0x21)
> +DEFOP (const8, 1, 8, 0, 1, 0x22)
> +DEFOP (const16, 2, 16, 0, 1, 0x23)
> +DEFOP (const32, 4, 32, 0, 1, 0x24)
> +DEFOP (const64, 8, 64, 0, 1, 0x25)
> +DEFOP (reg, 2, 0, 0, 1, 0x26)
> +DEFOP (end, 0, 0, 0, 0, 0x27)
> +DEFOP (dup, 0, 0, 1, 2, 0x28)
> +DEFOP (pop, 0, 0, 1, 0, 0x29)
> +DEFOP (zero_ext, 1, 0, 1, 1, 0x2a)
> +DEFOP (swap, 0, 0, 2, 2, 0x2b)
> +DEFOP (getv, 2, 0, 0, 1, 0x2c)
> +DEFOP (setv, 2, 0, 0, 1, 0x2d)
> +DEFOP (tracev, 2, 0, 0, 1, 0x2e)
> +/* We need something here just to make the tables come out ok.  */
> +DEFOP (invalid, 0, 0, 0, 0, 0x2f)
> +DEFOP (trace16, 2, 0, 1, 1, 0x30)
> +/* The "consumed" number for pick is wrong, but there's no way to
> +   express the right thing.  */
> +DEFOP (pick, 1, 0, 0, 1, 0x31)
> +DEFOP (rot, 0, 0, 3, 3, 0x32)
> diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c
> index 660cf3c..70e77a5 100644
> --- a/gdb/gdbserver/tracepoint.c
> +++ b/gdb/gdbserver/tracepoint.c
> @@ -470,111 +470,19 @@ write_inferior_uinteger (CORE_ADDR symaddr, unsigned int val)
>
>  enum gdb_agent_op
>   {
> -    gdb_agent_op_float = 0x01,
> -    gdb_agent_op_add = 0x02,
> -    gdb_agent_op_sub = 0x03,
> -    gdb_agent_op_mul = 0x04,
> -    gdb_agent_op_div_signed = 0x05,
> -    gdb_agent_op_div_unsigned = 0x06,
> -    gdb_agent_op_rem_signed = 0x07,
> -    gdb_agent_op_rem_unsigned = 0x08,
> -    gdb_agent_op_lsh = 0x09,
> -    gdb_agent_op_rsh_signed = 0x0a,
> -    gdb_agent_op_rsh_unsigned = 0x0b,
> -    gdb_agent_op_trace = 0x0c,
> -    gdb_agent_op_trace_quick = 0x0d,
> -    gdb_agent_op_log_not = 0x0e,
> -    gdb_agent_op_bit_and = 0x0f,
> -    gdb_agent_op_bit_or = 0x10,
> -    gdb_agent_op_bit_xor = 0x11,
> -    gdb_agent_op_bit_not = 0x12,
> -    gdb_agent_op_equal = 0x13,
> -    gdb_agent_op_less_signed = 0x14,
> -    gdb_agent_op_less_unsigned = 0x15,
> -    gdb_agent_op_ext = 0x16,
> -    gdb_agent_op_ref8 = 0x17,
> -    gdb_agent_op_ref16 = 0x18,
> -    gdb_agent_op_ref32 = 0x19,
> -    gdb_agent_op_ref64 = 0x1a,
> -    gdb_agent_op_ref_float = 0x1b,
> -    gdb_agent_op_ref_double = 0x1c,
> -    gdb_agent_op_ref_long_double = 0x1d,
> -    gdb_agent_op_l_to_d = 0x1e,
> -    gdb_agent_op_d_to_l = 0x1f,
> -    gdb_agent_op_if_goto = 0x20,
> -    gdb_agent_op_goto = 0x21,
> -    gdb_agent_op_const8 = 0x22,
> -    gdb_agent_op_const16 = 0x23,
> -    gdb_agent_op_const32 = 0x24,
> -    gdb_agent_op_const64 = 0x25,
> -    gdb_agent_op_reg = 0x26,
> -    gdb_agent_op_end = 0x27,
> -    gdb_agent_op_dup = 0x28,
> -    gdb_agent_op_pop = 0x29,
> -    gdb_agent_op_zero_ext = 0x2a,
> -    gdb_agent_op_swap = 0x2b,
> -    gdb_agent_op_getv = 0x2c,
> -    gdb_agent_op_setv = 0x2d,
> -    gdb_agent_op_tracev = 0x2e,
> -    gdb_agent_op_trace16 = 0x30,
> -    gdb_agent_op_pick = 0x31,
> -    gdb_agent_op_rot = 0x32,
> +#define DEFOP(NAME, SIZE, DATA_SIZE, CONSUMED, PRODUCED, VALUE)  \
> +    gdb_agent_op_ ## NAME = VALUE,
> +#include "ax.def"
> +#undef DEFOP
>     gdb_agent_op_last
>   };
>
>  static const char *gdb_agent_op_names [gdb_agent_op_last] =
>   {
> -    "?undef?",
> -    "float",
> -    "add",
> -    "sub",
> -    "mul",
> -    "div_signed",
> -    "div_unsigned",
> -    "rem_signed",
> -    "rem_unsigned",
> -    "lsh",
> -    "rsh_signed",
> -    "rsh_unsigned",
> -    "trace",
> -    "trace_quick",
> -    "log_not",
> -    "bit_and",
> -    "bit_or",
> -    "bit_xor",
> -    "bit_not",
> -    "equal",
> -    "less_signed",
> -    "less_unsigned",
> -    "ext",
> -    "ref8",
> -    "ref16",
> -    "ref32",
> -    "ref64",
> -    "ref_float",
> -    "ref_double",
> -    "ref_long_double",
> -    "l_to_d",
> -    "d_to_l",
> -    "if_goto",
> -    "goto",
> -    "const8",
> -    "const16",
> -    "const32",
> -    "const64",
> -    "reg",
> -    "end",
> -    "dup",
> -    "pop",
> -    "zero_ext",
> -    "swap",
> -    "getv",
> -    "setv",
> -    "tracev",
> -    "?undef?",
> -    "trace16",
> -    "pick",
> -    "rot"
> +    "?undef?"
> +#define DEFOP(NAME, SIZE, DATA_SIZE, CONSUMED, PRODUCED, VALUE)  , # NAME
> +#include "ax.def"
> +#undef DEFOP
>   };
>
>  struct agent_expr
>