Hello Richard,
eons ago Richard wrote:
>
> Expand the definition of "not present" to include "should not be present".
> This means we can simplify the logic surrounding the generic tcg opcodes
> for which the host backend ought not be providing definitions.
>
> Signed-off-by: Richard Henderson <address@hidden>
> ---
> tcg/tcg-opc.h | 26 +++++++++++++++-----------
> tcg/tcg.c | 4 +---
> tcg/tcg.h | 3 ++-
> 3 files changed, 18 insertions(+), 15 deletions(-)
>
> diff --git a/tcg/tcg-opc.h b/tcg/tcg-opc.h
> index db5e6e5..83f7147 100644
> --- a/tcg/tcg-opc.h
> +++ b/tcg/tcg-opc.h
> @@ -27,17 +27,21 @@
> */
>
> /* predefined ops */
> -DEF(end, 0, 0, 0, 0) /* must be kept first */
> -DEF(nop, 0, 0, 0, 0)
> -DEF(nop1, 0, 0, 1, 0)
> -DEF(nop2, 0, 0, 2, 0)
> -DEF(nop3, 0, 0, 3, 0)
> -DEF(nopn, 0, 0, 1, 0) /* variable number of parameters */
> +DEF(end, 0, 0, 0, TCG_OPF_NOT_PRESENT) /* must be kept first */
> +DEF(nop, 0, 0, 0, TCG_OPF_NOT_PRESENT)
> +DEF(nop1, 0, 0, 1, TCG_OPF_NOT_PRESENT)
> +DEF(nop2, 0, 0, 2, TCG_OPF_NOT_PRESENT)
> +DEF(nop3, 0, 0, 3, TCG_OPF_NOT_PRESENT)
>
> -DEF(discard, 1, 0, 0, 0)
> +/* variable number of parameters */
> +DEF(nopn, 0, 0, 1, TCG_OPF_NOT_PRESENT)
> +
> +DEF(discard, 1, 0, 0, TCG_OPF_NOT_PRESENT)
> +DEF(set_label, 0, 0, 1, TCG_OPF_BB_END | TCG_OPF_NOT_PRESENT)
> +
> +/* variable number of parameters */
> +DEF(call, 0, 1, 2, TCG_OPF_CALL_CLOBBER | TCG_OPF_NOT_PRESENT)
If TCG_OPF_NOT_PRESENT is supposed to mark opcodes that should not be
implemented by the target, then setting TCG_OPF_NOT_PRESENT for 'call' seems
wrong to me.
This will actually trigger the code in the ifdef CONFIG_DEBUG_TCG and cause an
assert to fail.
>
> -DEF(set_label, 0, 0, 1, TCG_OPF_BB_END)
> -DEF(call, 0, 1, 2, TCG_OPF_CALL_CLOBBER) /* variable number of parameters */
> DEF(br, 0, 0, 1, TCG_OPF_BB_END)
>
> #define IMPL(X) (__builtin_constant_p(X) && !(X) ? TCG_OPF_NOT_PRESENT : 0)
> @@ -166,9 +170,9 @@ DEF(muls2_i64, 2, 2, 0, IMPL64 |
> IMPL(TCG_TARGET_HAS_muls2_i64))
>
> /* QEMU specific */
> #if TARGET_LONG_BITS > TCG_TARGET_REG_BITS
> -DEF(debug_insn_start, 0, 0, 2, 0)
> +DEF(debug_insn_start, 0, 0, 2, TCG_OPF_NOT_PRESENT)
> #else
> -DEF(debug_insn_start, 0, 0, 1, 0)
> +DEF(debug_insn_start, 0, 0, 1, TCG_OPF_NOT_PRESENT)
> #endif
> DEF(exit_tb, 0, 0, 1, TCG_OPF_BB_END)
> DEF(goto_tb, 0, 0, 1, TCG_OPF_BB_END)
> diff --git a/tcg/tcg.c b/tcg/tcg.c
> index 1d8099c..c7e6567 100644
> --- a/tcg/tcg.c
> +++ b/tcg/tcg.c
> @@ -1160,9 +1160,7 @@ void tcg_add_target_add_op_defs(const TCGTargetOpDef
> *tdefs)
> i = 0;
> for (op = 0; op < ARRAY_SIZE(tcg_op_defs); op++) {
> const TCGOpDef *def = &tcg_op_defs[op];
> - if (op < INDEX_op_call
> - || op == INDEX_op_debug_insn_start
> - || (def->flags & TCG_OPF_NOT_PRESENT)) {
> + if (def->flags & TCG_OPF_NOT_PRESENT) {
this will trigger for op = 'call'
> /* Wrong entry in op definitions? */
> if (def->used) {
> fprintf(stderr, "Invalid op definition for %s\n", def->name);
> diff --git a/tcg/tcg.h b/tcg/tcg.h
> index df375cf..72b694f 100644
> --- a/tcg/tcg.h
> +++ b/tcg/tcg.h
> @@ -593,7 +593,8 @@ enum {
> TCG_OPF_SIDE_EFFECTS = 0x04,
> /* Instruction operands are 64-bits (otherwise 32-bits). */
> TCG_OPF_64BIT = 0x08,
> - /* Instruction is optional and not implemented by the host. */
> + /* Instruction is optional and not implemented by the host, or insn
> + is generic and should not be implemened by the host. */
> TCG_OPF_NOT_PRESENT = 0x10,
> };
>
> --
> 1.8.1.4
