Re: [PATCH v2 06/10] target/hexagon: prepare input for the idef-parser

[Richard Henderson]

On 2/25/21 7:18 AM, Alessandro Di Federico wrote:
> 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>
> ---
>  target/hexagon/gen_idef_parser_funcs.py | 114 ++++++++++++++++
>  target/hexagon/idef-parser/macros.inc   | 166 ++++++++++++++++++++++++
>  target/hexagon/idef-parser/prepare      |  33 +++++
>  target/hexagon/meson.build              |  18 +++
>  4 files changed, 331 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 
> b/target/hexagon/gen_idef_parser_funcs.py
> new file mode 100644
> index 0000000000..6fb3659201
> --- /dev/null
> +++ b/target/hexagon/gen_idef_parser_funcs.py
> @@ -0,0 +1,114 @@
> +#!/usr/bin/env python3
> +
> +##
> +##  Copyright(c) 2019-2020 rev.ng Srls. All Rights Reserved.
> +##
> +##  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 2 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/>.
> +##
> +
> +import sys
> +import re
> +import string
> +from io import StringIO
> +
> +import hex_common
> +
> +##
> +## Generate code to be fed to the idef_parser
> +##
> +## Consider A2_add:
> +##
> +##     Rd32=add(Rs32,Rt32), { RdV=RsV+RtV;}
> +##
> +## We produce:
> +##
> +##     A2_add(RdV, in RsV, in RtV) {
> +##       { RdV=RsV+RtV;}
> +##     }
> +##
> +## A2_add represents the instruction tag. Then we have a list of TCGv
> +## that the code generated by the parser can expect in input. Some of
> +## them are inputs ("in" prefix), while some others are outputs.
> +##
> +def main():
> +    hex_common.read_semantics_file(sys.argv[1])
> +    hex_common.read_attribs_file(sys.argv[2])
> +    hex_common.read_overrides_file(sys.argv[3])
> +    hex_common.calculate_attribs()
> +    tagregs = hex_common.get_tagregs()
> +    tagimms = hex_common.get_tagimms()
> +
> +    with open(sys.argv[4], 'w') as f:
> +        f.write('#include "macros.inc"\n\n')
> +
> +        for tag in hex_common.tags:
> +            ## Skip the priv instructions
> +            if ( "A_PRIV" in hex_common.attribdict[tag] ) :
> +                continue
> +            ## Skip the guest instructions
> +            if ( "A_GUEST" in hex_common.attribdict[tag] ) :
> +                continue
> +            ## Skip instructions using switch
> +            if ( tag in {'S4_vrcrotate_acc', 'S4_vrcrotate'} ) :
> +                continue
> +            ## Skip trap instructions
> +            if ( tag in {'J2_trap0', 'J2_trap1'} ) :
> +                continue
> +            ## Skip 128-bit instructions
> +            if ( tag in {'A7_croundd_ri', 'A7_croundd_rr'} ) :
> +                continue
> +            ## Skip other unsupported instructions
> +            if ( tag.startswith('S2_cabacdecbin') ) :
> +                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")
> +
> +            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)))
> +            f.write("    %s\n" % hex_common.semdict[tag])
> +            f.write("}\n\n")
> +
> +if __name__ == "__main__":
> +    main()
> diff --git a/target/hexagon/idef-parser/macros.inc 
> b/target/hexagon/idef-parser/macros.inc
> new file mode 100644
> index 0000000000..719bebaee3
> --- /dev/null
> +++ b/target/hexagon/idef-parser/macros.inc
> @@ -0,0 +1,166 @@
> +/*
> + * Copyright(c) 2019-2020 rev.ng Srls. 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/>.
> + */
> +
> +/* Copy rules */
> +#define fLSBOLD(VAL) (fGETBIT(0, VAL))
> +#define fSATH(VAL) fSATN(16, VAL)
> +#define fSATUH(VAL) fSATUN(16, VAL)
> +#define fVSATH(VAL) fVSATN(16, VAL)
> +#define fVSATUH(VAL) fVSATUN(16, VAL)
> +#define fSATUB(VAL) fSATUN(8, VAL)
> +#define fSATB(VAL) fSATN(8, VAL)
> +#define fVSATUB(VAL) fVSATUN(8, VAL)
> +#define fVSATB(VAL) fVSATN(8, VAL)
> +#define fCALL(A) fWRITE_LR(fREAD_NPC()); fWRITE_NPC(A);
> +#define fCALLR(A) fWRITE_LR(fREAD_NPC()); fWRITE_NPC(A);
> +#define fCAST2_8s(A) fSXTN(16, 64, A)
> +#define fCAST2_8u(A) fZXTN(16, 64, A)
> +#define fCAST8S_16S(A) (fSXTN(64, 128, A))
> +#define fCAST16S_8S(A) (fSXTN(128, 64, A))
> +#define fVSATW(A) fVSATN(32, fCAST8_8s(A))
> +#define fSATW(A) fSATN(32, fCAST8_8s(A))
> +#define fVSAT(A) fVSATN(32, A)
> +#define fSAT(A) fSATN(32, A)
> +
> +/* Ease parsing */
> +#define f8BITSOF(VAL) ((VAL) ? 0xff : 0x00)
> +#define fREAD_GP() (Constant_extended ? (0) : GP)
> +#define fCLIP(DST, SRC, U) (DST = fMIN((1 << U) - 1, fMAX(SRC, -(1 << U))))

I guess this is what's in the manual, but my reading of this expression is
"saturate", not "clip".  How does it differ from

  fSATN(U, SRC)

?

> +#define fCARRY_FROM_ADD(A, B, C)                                        \
> +    fGETUWORD(1,                                                        \
> +              fGETUWORD(1, A) +                                         \
> +              fGETUWORD(1, B) +                                         \
> +              fGETUWORD(1,                                              \
> +                        fGETUWORD(0, A) +                               \
> +                        fGETUWORD(0, B) + C))

Hmm.  FWIW, it's probably worth letting this pass through to bison so that you
can expand with tcg_gen_add2.

> +#define fADDSAT64(DST, A, B)                                            \
> +        __a = fCAST8u(A);                                               \
> +        __b = fCAST8u(B);                                               \
> +        __sum = __a + __b;                                              \
> +        __xor = __a ^ __b;                                              \
> +        __mask = 0x8000000000000000ULL;                                 \
> +        if (__xor & __mask) {                                           \
> +            DST = __sum;                                                \
> +        }                                                               \
> +        else if ((__a ^ __sum) & __mask) {                              \
> +            if (__sum & __mask) {                                       \
> +                DST = 0x7FFFFFFFFFFFFFFFLL;                             \
> +                fSET_OVERFLOW();                                        \
> +            } else {                                                    \
> +                DST = 0x8000000000000000ULL;                            \
> +                fSET_OVERFLOW();    

Why not squash some of the subexpressions?

  if ((__a ^ __b) | ~(__a ^ sum)) & __mask) {
    DST = __sum;
  } else {
    DST = ((__sum & __mask) >> 63) + __mask;
    fSET_OVERFLOW();
  }


> +/* Negation operator */
> +#define fLSBOLDNOT(VAL) (!fGETBIT(0, VAL))

  fGETBIT(0, ~VAL) ?

> +# 2. Transform
> +#
> +#        condition ? A = B : A = C
> +#
> +#    in
> +#
> +#        A = (condition ? B : C)
> +#
> +# 3. Remove comments (starting with "#")
> +cpp "$@" | sed 's/\(\s*[{;]\)\s*\([^;?]*\) ? 
> (\([^;=]*\)=\([^;)]*\))\s*:\s*([^;=]*=\([^;)]*\));/\1 \3 = (\2) ? \4 : \5;/' 
> | grep -v '^#'

Wow, that is a really ugly regexp.  It screams for handling this in the parser
instead.  Which honestly doesn't seem that hard.


r~