Re: [Qemu-devel] [PATCH 11/67] target/arm: Add stubs for aa32 decodetree

[Aleksandar Markovic <aleksandar.m.mail@gmail.com>]

On Fri, Jul 26, 2019 at 8:05 PM Richard Henderson <
richard.henderson@linaro.org> wrote:

> Add the infrastructure that will become the new decoder.
> No instructions adjusted so far.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/translate.c       | 45 +++++++++++++++++++++++++++++++++++-
>  target/arm/Makefile.objs     | 18 +++++++++++++++
>  target/arm/a32-uncond.decode | 23 ++++++++++++++++++
>  target/arm/a32.decode        | 23 ++++++++++++++++++
>  target/arm/t32.decode        | 20 ++++++++++++++++
>  5 files changed, 128 insertions(+), 1 deletion(-)
>  create mode 100644 target/arm/a32-uncond.decode
>  create mode 100644 target/arm/a32.decode
>  create mode 100644 target/arm/t32.decode
>
> diff --git a/target/arm/translate.c b/target/arm/translate.c
> index 36419025db..4738b91957 100644
> --- a/target/arm/translate.c
> +++ b/target/arm/translate.c
> @@ -7715,6 +7715,33 @@ static void arm_skip_unless(DisasContext *s,
> uint32_t cond)
>      }
>  }
>
> +/*
> + * Include the generated decoders.
> + * Note that the T32 decoder reuses some of the trans_* functions
> + * initially declared by the A32 decoder, which results in duplicate
> + * declaration warnings.  Suppress them.
> + */
> +
> +#ifdef CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE
> +# pragma GCC diagnostic push
> +# pragma GCC diagnostic ignored "-Wredundant-decls"
> +# ifdef __clang__
> +#  pragma GCC diagnostic ignored "-Wtypedef-redefinition"
> +# endif
> +#endif
> +
>

This looks more like a "band aid" solution rather than the right one.

I find it surprising that in spite of ever-growing complexity and numerous
refinements of the decodetree module, it still generates code that causes
these complaints of the compiler.

Regards,
Aleksandar



> +#include "decode-a32.inc.c"
> +#include "decode-a32-uncond.inc.c"
> +#include "decode-t32.inc.c"
> +
> +#ifdef CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE
> +# pragma GCC diagnostic pop
> +#endif
> +
> +/*
> + * Legacy decoder.
> + */
> +
>  static void disas_arm_insn(DisasContext *s, unsigned int insn)
>  {
>      unsigned int cond, val, op1, i, shift, rm, rs, rn, rd, sh;
> @@ -7733,7 +7760,8 @@ static void disas_arm_insn(DisasContext *s, unsigned
> int insn)
>          return;
>      }
>      cond = insn >> 28;
> -    if (cond == 0xf){
> +
> +    if (cond == 0xf) {
>          /* In ARMv3 and v4 the NV condition is UNPREDICTABLE; we
>           * choose to UNDEF. In ARMv5 and above the space is used
>           * for miscellaneous unconditional instructions.
> @@ -7741,6 +7769,11 @@ static void disas_arm_insn(DisasContext *s,
> unsigned int insn)
>          ARCH(5);
>
>          /* Unconditional instructions.  */
> +        if (disas_a32_uncond(s, insn)) {
> +            return;
> +        }
> +        /* fall back to legacy decoder */
> +
>          if (((insn >> 25) & 7) == 1) {
>              /* NEON Data processing.  */
>              if (!arm_dc_feature(s, ARM_FEATURE_NEON)) {
> @@ -7953,6 +7986,11 @@ static void disas_arm_insn(DisasContext *s,
> unsigned int insn)
>
>      arm_skip_unless(s, cond);
>
> +    if (disas_a32(s, insn)) {
> +        return;
> +    }
> +    /* fall back to legacy decoder */
> +
>      if ((insn & 0x0f900000) == 0x03000000) {
>          if ((insn & (1 << 21)) == 0) {
>              ARCH(6T2);
> @@ -9440,6 +9478,11 @@ static void disas_thumb2_insn(DisasContext *s,
> uint32_t insn)
>          ARCH(6T2);
>      }
>
> +    if (disas_t32(s, insn)) {
> +        return;
> +    }
> +    /* fall back to legacy decoder */
> +
>      rn = (insn >> 16) & 0xf;
>      rs = (insn >> 12) & 0xf;
>      rd = (insn >> 8) & 0xf;
> diff --git a/target/arm/Makefile.objs b/target/arm/Makefile.objs
> index 5cafc1eb6c..7806b4dac0 100644
> --- a/target/arm/Makefile.objs
> +++ b/target/arm/Makefile.objs
> @@ -28,9 +28,27 @@ target/arm/decode-vfp-uncond.inc.c:
> $(SRC_PATH)/target/arm/vfp-uncond.decode $(D
>           $(PYTHON) $(DECODETREE) --static-decode disas_vfp_uncond -o $@
> $<,\
>           "GEN", $(TARGET_DIR)$@)
>
> +target/arm/decode-a32.inc.c: $(SRC_PATH)/target/arm/a32.decode
> $(DECODETREE)
> +       $(call quiet-command,\
> +         $(PYTHON) $(DECODETREE) --static-decode disas_a32 -o $@ $<,\
> +         "GEN", $(TARGET_DIR)$@)
> +
> +target/arm/decode-a32-uncond.inc.c:
> $(SRC_PATH)/target/arm/a32-uncond.decode $(DECODETREE)
> +       $(call quiet-command,\
> +         $(PYTHON) $(DECODETREE) --static-decode disas_a32_uncond -o $@
> $<,\
> +         "GEN", $(TARGET_DIR)$@)
> +
> +target/arm/decode-t32.inc.c: $(SRC_PATH)/target/arm/t32.decode
> $(DECODETREE)
> +       $(call quiet-command,\
> +         $(PYTHON) $(DECODETREE) --static-decode disas_t32 -o $@ $<,\
> +         "GEN", $(TARGET_DIR)$@)
> +
>  target/arm/translate-sve.o: target/arm/decode-sve.inc.c
>  target/arm/translate.o: target/arm/decode-vfp.inc.c
>  target/arm/translate.o: target/arm/decode-vfp-uncond.inc.c
> +target/arm/translate.o: target/arm/decode-a32.inc.c
> +target/arm/translate.o: target/arm/decode-a32-uncond.inc.c
> +target/arm/translate.o: target/arm/decode-t32.inc.c
>
>  obj-y += tlb_helper.o debug_helper.o
>  obj-y += translate.o op_helper.o
> diff --git a/target/arm/a32-uncond.decode b/target/arm/a32-uncond.decode
> new file mode 100644
> index 0000000000..8dee26d3b6
> --- /dev/null
> +++ b/target/arm/a32-uncond.decode
> @@ -0,0 +1,23 @@
> +# A32 unconditional instructions
> +#
> +#  Copyright (c) 2019 Linaro, Ltd
> +#
> +# This library is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU Lesser General Public
> +# License as published by the Free Software Foundation; either
> +# version 2 of the License, or (at your option) any later version.
> +#
> +# This library 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
> +# Lesser General Public License for more details.
> +#
> +# You should have received a copy of the GNU Lesser General Public
> +# License along with this library; if not, see <
> http://www.gnu.org/licenses/>.
> +
> +#
> +# This file is processed by scripts/decodetree.py
> +#
> +# All insns that have 0xf in insn[31:28] are decoded here.
> +# All of those that have a COND field in insn[31:28] are in a32.decode
> +#
> diff --git a/target/arm/a32.decode b/target/arm/a32.decode
> new file mode 100644
> index 0000000000..2d84a02861
> --- /dev/null
> +++ b/target/arm/a32.decode
> @@ -0,0 +1,23 @@
> +# A32 conditional instructions
> +#
> +#  Copyright (c) 2019 Linaro, Ltd
> +#
> +# This library is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU Lesser General Public
> +# License as published by the Free Software Foundation; either
> +# version 2 of the License, or (at your option) any later version.
> +#
> +# This library 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
> +# Lesser General Public License for more details.
> +#
> +# You should have received a copy of the GNU Lesser General Public
> +# License along with this library; if not, see <
> http://www.gnu.org/licenses/>.
> +
> +#
> +# This file is processed by scripts/decodetree.py
> +#
> +# All of the insn that have a COND field in insn[31:28] are here.
> +# All insns that have 0xf in insn[31:28] are in a32u.decode.
> +#
> diff --git a/target/arm/t32.decode b/target/arm/t32.decode
> new file mode 100644
> index 0000000000..ac01fb6958
> --- /dev/null
> +++ b/target/arm/t32.decode
> @@ -0,0 +1,20 @@
> +# Thumb2 instructions
> +#
> +#  Copyright (c) 2019 Linaro, Ltd
> +#
> +# This library is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU Lesser General Public
> +# License as published by the Free Software Foundation; either
> +# version 2 of the License, or (at your option) any later version.
> +#
> +# This library 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
> +# Lesser General Public License for more details.
> +#
> +# You should have received a copy of the GNU Lesser General Public
> +# License along with this library; if not, see <
> http://www.gnu.org/licenses/>.
> +
> +#
> +# This file is processed by scripts/decodetree.py
> +#
> --
> 2.17.1
>
>
>