RE: [PATCH v7 07/13] target/hexagon: prepare input for the idef-parser

[Taylor Simpson <tsimpson@quicinc.com>]

> -----Original Message-----
> From: Anton Johansson <anjo@rev.ng>
> Sent: Friday, December 17, 2021 2:01 AM
> To: qemu-devel@nongnu.org
> Cc: ale@rev.ng; Taylor Simpson <tsimpson@quicinc.com>; Brian Cain
> <bcain@quicinc.com>; babush@rev.ng; nizzo@rev.ng;
> richard.henderson@linaro.org
> Subject: [PATCH v7 07/13] target/hexagon: prepare input for the idef-parser
> 
> From: Alessandro Di Federico <ale@rev.ng>
> 
> Introduce infrastructure necessary to produce a file suitable for being parsed
> by the idef-parser.
> 
> Signed-off-by: Alessandro Di Federico <ale@rev.ng>
> Signed-off-by: Anton Johansson <anjo@rev.ng>
> ---
>  target/hexagon/gen_idef_parser_funcs.py | 123
> +++++++++++++++++++++
>  target/hexagon/idef-parser/macros.inc   | 140
> ++++++++++++++++++++++++
>  target/hexagon/idef-parser/prepare      |  24 ++++
>  target/hexagon/meson.build              |  16 +++
>  4 files changed, 303 insertions(+)
>  create mode 100644 target/hexagon/gen_idef_parser_funcs.py
>  create mode 100644 target/hexagon/idef-parser/macros.inc
>  create mode 100755 target/hexagon/idef-parser/prepare
> 
> diff --git a/target/hexagon/gen_idef_parser_funcs.py
> +            ## Skip other unsupported instructions
> +            if ( tag.startswith('S2_cabacdecbin') ) :
> +                continue
> +            if ( tag.startswith('A5_ACS') ) :
> +                continue

Not startswith, just
            if (tag == 'S2_cabacdecbin' or tag == 'A5_ACS'):
                continue

> +            if ( tag.startswith('Y') ) :
> +                continue
> +            if ( tag.startswith('V6_') ) :
> +                continue
> +            if ( tag.startswith('F') ) :
> +                continue
> +            if ( tag.endswith('_locked') ) :
> +                continue
> +
> +            regs = tagregs[tag]
> +            imms = tagimms[tag]
> +
> +            arguments = []
> +            if hex_common.need_ea(tag):
> +                arguments.append("EA")

It's a hack to add EA to the arguments list and then have special handling for it in the parser
                  /*
                   * The effective address EA is the only variable passed to
                   * function which needs to be "created".
                   */
                  if (c->inst.EA) {
                      gen_varid_allocate(c, &@1, c->inst.EA, 32, SIGNED);
                  }
Instead, put in a declaration inside the body - see below.


> +            for regtype,regid,toss,numregs in regs:
> +                prefix = "in " if hex_common.is_read(regid) else ""
> +
> +                is_pair = hex_common.is_pair(regid)
> +                is_single_old = (hex_common.is_single(regid)
> +                                 and hex_common.is_old_val(regtype, regid, tag))
> +                is_single_new = (hex_common.is_single(regid)
> +                                 and hex_common.is_new_val(regtype,
> + regid, tag))
> +
> +                if is_pair or is_single_old:
> +                    arguments.append("%s%s%sV" % (prefix, regtype, regid))
> +                elif is_single_new:
> +                    arguments.append("%s%s%sN" % (prefix, regtype, regid))
> +                else:
> +                    print("Bad register parse:
> + ",regtype,regid,toss,numregs)
> +
> +            for immlett,bits,immshift in imms:
> +                arguments.append(hex_common.imm_name(immlett))
> +
> +            f.write("%s(%s) {\n" % (tag, ", ".join(arguments)))

Put the declaration here inside the body.
            if hex_common.need_ea(tag):
                f.write("    size4u_t EA;\n")

> +            f.write("    %s\n" % hex_common.semdict[tag])
> +            f.write("}\n\n")
> +
> +if __name__ == "__main__":
> +    main()



> diff --git a/target/hexagon/idef-parser/prepare b/target/hexagon/idef-
> parser/prepare
> new file mode 100755
> index 0000000000..6377513dc5
> --- /dev/null
> +++ b/target/hexagon/idef-parser/prepare
> @@ -0,0 +1,24 @@
> +#!/bin/bash
> +
> +#
> +# Copyright(c) 2019-2021 rev.ng Labs Srl. All Rights Reserved.
> +#
> +# 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/>.
> +#

I'm not a lawyer, but why are you marking this with LGPL?

> +
> +set -e
> +set -o pipefail
> +
> +# Run the preprocessor and drop comments cpp "$@"


> diff --git a/target/hexagon/meson.build b/target/hexagon/meson.build
> index b61243103f..236c3a3ffa 100644
> --- a/target/hexagon/meson.build
> +++ b/target/hexagon/meson.build
> @@ -21,6 +21,7 @@ hex_common_py = 'hex_common.py'
>  attribs_def = meson.current_source_dir() / 'attribs_def.h.inc'
>  gen_tcg_h = meson.current_source_dir() / 'gen_tcg.h'
>  gen_tcg_hvx_h = meson.current_source_dir() / 'gen_tcg_hvx.h'
> +idef_parser_dir = meson.current_source_dir() / 'idef-parser'
> 
>  #
>  #  Step 1
> @@ -179,4 +180,19 @@ hexagon_ss.add(files(
>      'mmvec/system_ext_mmvec.c',
>  ))
> 
> +idef_parser_input_generated = custom_target(
> +    'idef_parser_input.h.inc',
> +    output: 'idef_parser_input.h.inc',
> +    depends: [semantics_generated],
> +    depend_files: [hex_common_py],
> +    command: [python, files('gen_idef_parser_funcs.py'),
> +semantics_generated, attribs_def, '@OUTPUT@'],
> +)
> +
> +idef_parser_input_generated_prep = custom_target(
> +    'idef_parser_input.preprocessed.h.inc',
> +    output: 'idef_parser_input.preprocessed.h.inc',
> +    input: idef_parser_input_generated,
> +    command: [idef_parser_dir / 'prepare', '@INPUT@', '-I' +
> +idef_parser_dir, '-o', '@OUTPUT@'],
> +)
> +
>  target_arch += {'hexagon': hexagon_ss}
> --
> 2.33.1