RE: [PATCH v5 05/14] Hexagon (target/hexagon) Analyze packet before generating TCG

[Taylor Simpson]

> -----Original Message-----
> From: Anton Johansson <anjo@rev.ng>
> Sent: Thursday, February 23, 2023 10:02 AM
> To: Taylor Simpson <tsimpson@quicinc.com>; qemu-devel@nongnu.org
> Cc: richard.henderson@linaro.org; philmd@linaro.org; ale@rev.ng; Brian Cain
> <bcain@quicinc.com>; Matheus Bernardino (QUIC)
> <quic_mathbern@quicinc.com>
> Subject: Re: [PATCH v5 05/14] Hexagon (target/hexagon) Analyze packet
> before generating TCG
> 
> On 1/31/23 23:56, Taylor Simpson wrote:
> > diff --git a/target/hexagon/gen_analyze_funcs.py
> > b/target/hexagon/gen_analyze_funcs.py
> > new file mode 100755
> > index 0000000000..c45696bec8
> > --- /dev/null
> > +++ b/target/hexagon/gen_analyze_funcs.py
> > @@ -0,0 +1,237 @@
> > +#!/usr/bin/env python3
> > +
> > +##
> > +##  Copyright(c) 2022-2023 Qualcomm Innovation Center, Inc. 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
> > +import hex_common
> > +
> > +##
> > +## Helpers for gen_analyze_func
> > +##
> > +def is_predicated(tag):
> > +    return 'A_CONDEXEC' in hex_common.attribdict[tag]
> > +
> > +def analyze_opn_old(f, tag, regtype, regid, regno):
> > +    regN = "%s%sN" % (regtype, regid)
> > +    predicated = "true" if is_predicated(tag) else "false"
> > +    if (regtype == "R"):
> > +        if (regid in {"ss", "tt"}):
> > +            f.write("//    const int %s = insn->regno[%d];\n" % \
> > +                (regN, regno))
> > +        elif (regid in {"dd", "ee", "xx", "yy"}):
> > +            f.write("    const int %s = insn->regno[%d];\n" % (regN, 
> > regno))
> > +            f.write("    ctx_log_reg_write_pair(ctx, %s, %s);\n" % \
> > +                (regN, predicated))
> > +        elif (regid in {"s", "t", "u", "v"}):
> > +            f.write("//    const int %s = insn->regno[%d];\n" % \
> > +                (regN, regno))
> > +        elif (regid in {"d", "e", "x", "y"}):
> > +            f.write("    const int %s = insn->regno[%d];\n" % (regN, 
> > regno))
> > +            f.write("    ctx_log_reg_write(ctx, %s, %s);\n" % \
> > +                (regN, predicated))
> > +        else:
> > +            print("Bad register parse: ", regtype, regid)
> > +    elif (regtype == "P"):
> > +        if (regid in {"s", "t", "u", "v"}):
> > +            f.write("//    const int %s = insn->regno[%d];\n" % \
> > +                (regN, regno))
> > +        elif (regid in {"d", "e", "x"}):
> > +            f.write("    const int %s = insn->regno[%d];\n" % (regN, 
> > regno))
> > +            f.write("    ctx_log_pred_write(ctx, %s);\n" % (regN))
> > +        else:
> > +            print("Bad register parse: ", regtype, regid)
> > +    elif (regtype == "C"):
> > +        if (regid == "ss"):
> > +            f.write("//    const int %s = insn->regno[%d] + 
> > HEX_REG_SA0;\n" % \
> > +                (regN, regno))
> > +        elif (regid == "dd"):
> > +            f.write("    const int %s = insn->regno[%d] + HEX_REG_SA0;\n" 
> > % \
> > +                (regN, regno))
> > +            f.write("    ctx_log_reg_write_pair(ctx, %s, %s);\n" % \
> > +                (regN, predicated))
> > +        elif (regid == "s"):
> > +            f.write("//    const int %s = insn->regno[%d] + 
> > HEX_REG_SA0;\n" % \
> > +                (regN, regno))
> > +        elif (regid == "d"):
> > +            f.write("    const int %s = insn->regno[%d] + HEX_REG_SA0;\n" 
> > % \
> > +                (regN, regno))
> > +            f.write("    ctx_log_reg_write(ctx, %s, %s);\n" % \
> > +                (regN, predicated))
> > +        else:
> > +            print("Bad register parse: ", regtype, regid)
> > +    elif (regtype == "M"):
> > +        if (regid == "u"):
> > +            f.write("//    const int %s = insn->regno[%d];\n"% \
> > +                (regN, regno))
> > +        else:
> > +            print("Bad register parse: ", regtype, regid)
> > +    elif (regtype == "V"):
> > +        if (regid in {"dd", "xx"}):
> > +            f.write("//    const int %s = insn->regno[%d];\n" %\
> > +                (regN, regno))
> > +        elif (regid in {"uu", "vv"}):
> > +            f.write("//    const int %s = insn->regno[%d];\n" % \
> > +                (regN, regno))
> > +        elif (regid in {"s", "u", "v", "w"}):
> > +            f.write("//    const int %s = insn->regno[%d];\n" % \
> > +                (regN, regno))
> > +        elif (regid in {"d", "x", "y"}):
> > +            f.write("//    const int %s = insn->regno[%d];\n" % \
> > +                (regN, regno))
> > +        else:
> > +            print("Bad register parse: ", regtype, regid)
> > +    elif (regtype == "Q"):
> > +        if (regid in {"d", "e", "x"}):
> > +            f.write("//    const int %s = insn->regno[%d];\n" % \
> > +                (regN, regno))
> > +        elif (regid in {"s", "t", "u", "v"}):
> > +            f.write("//    const int %s = insn->regno[%d];\n" % \
> > +                (regN, regno))
> > +        else:
> > +            print("Bad register parse: ", regtype, regid)
> > +    elif (regtype == "G"):
> > +        if (regid in {"dd"}):
> > +            f.write("//    const int %s = insn->regno[%d];\n" % \
> > +                (regN, regno))
> > +        elif (regid in {"d"}):
> > +            f.write("//    const int %s = insn->regno[%d];\n" % \
> > +                (regN, regno))
> > +        elif (regid in {"ss"}):
> > +            f.write("//    const int %s = insn->regno[%d];\n" % \
> > +                (regN, regno))
> > +        elif (regid in {"s"}):
> > +            f.write("//    const int %s = insn->regno[%d];\n" % \
> > +                (regN, regno))
> > +        else:
> > +            print("Bad register parse: ", regtype, regid)
> > +    elif (regtype == "S"):
> > +        if (regid in {"dd"}):
> > +            f.write("//    const int %s = insn->regno[%d];\n" % \
> > +                (regN, regno))
> > +        elif (regid in {"d"}):
> > +            f.write("//    const int %s = insn->regno[%d];\n" % \
> > +                (regN, regno))
> > +        elif (regid in {"ss"}):
> > +            f.write("//    const int %s = insn->regno[%d];\n" % \
> > +                (regN, regno))
> > +        elif (regid in {"s"}):
> > +            f.write("//    const int %s = insn->regno[%d];\n" % \
> > +                (regN, regno))
> > +        else:
> > +            print("Bad register parse: ", regtype, regid)
> > +    else:
> > +        print("Bad register parse: ", regtype, regid)
> > +
> > +def analyze_opn_new(f, tag, regtype, regid, regno):
> > +    regN = "%s%sN" % (regtype, regid)
> > +    if (regtype == "N"):
> > +        if (regid in {"s", "t"}):
> > +            f.write("//    const int %s = insn->regno[%d];\n" % \
> > +                (regN, regno))
> > +        else:
> > +            print("Bad register parse: ", regtype, regid)
> > +    elif (regtype == "P"):
> > +        if (regid in {"t", "u", "v"}):
> > +            f.write("//    const int %s = insn->regno[%d];\n" % \
> > +                (regN, regno))
> > +        else:
> > +            print("Bad register parse: ", regtype, regid)
> > +    elif (regtype == "O"):
> > +        if (regid == "s"):
> > +            f.write("//    const int %s = insn->regno[%d];\n" % \
> > +                (regN, regno))
> > +        else:
> > +            print("Bad register parse: ", regtype, regid)
> > +    else:
> > +        print("Bad register parse: ", regtype, regid)
> 
> For analyze_opn_new and analyze_opn_old consider condensing the if/else
> statements since most branches are very similar. I find something like the
> following more readable
> 
>      def analyze_opn_old(f, tag, regtype, regid, regno):
>          regN = "%s%sN" % (regtype, regid)
>          predicated = "true" if is_predicated(tag) else "false"
>          pair = "_pair" if hex_common.is_pair(regid) else ""
>          if (regtype == "R" and regid in {"dd", "ee", "xx", "yy", "d", "e", 
> "x", "y"}):
>              f.write("    const int %s = insn->regno[%d];\n" % (regN,
> regno))
>              f.write("    ctx_log_reg_write%s(ctx, %s, %s);\n" % (pair, regN,
> predicated))
>          elif (regtype == "P" and regid in {"d", "e", "x"}):
>              f.write("    const int %s = insn->regno[%d];\n" % (regN,
> regno))
>              f.write("    ctx_log_pred_write(ctx, %s);\n" % (regN))
>          elif (regtype == "C" and regid in {"d", "dd"}):
>              f.write("    const int %s = insn->regno[%d] + HEX_REG_SA0;\n" %
> (regN, regno))
>              f.write("    ctx_log_reg_write%s(ctx, %s, %s);\n" % (pair, regN,
> predicated))
>          elif (regtype == "V" and regid in {"d", "x", "y", "dd", "xx"}):
>              newv = "EXT_DFL"
>              if (hex_common.is_new_result(tag)):
>                  newv = "EXT_NEW"
>              elif (hex_common.is_tmp_result(tag)):
>                  newv = "EXT_TMP"
>              f.write("    const int %s = insn->regno[%d];\n" % (regN,
> regno))
>              f.write("    ctx_log_vreg_write%s(ctx, %s, %s, %s);\n" % (pair, 
> regN,
> newv, predicated))
>          elif (regtype == "Q" and regid in {"d", "e", "x"}):
>              f.write("    const int %s = insn->regno[%d];\n" % (regN,
> regno))
>              f.write("    ctx_log_qreg_write(ctx, %s);\n" % (regN))
>          elif (regtype == "M"        and regid == "u" or
>                regtype in {"G", "S"} and regid in {"dd", "d", "ss", "s"} or
>                regtype in {"P", "Q"} and regid in {"s", "t", "u", "v"} or
>                regtype == "R"        and regid in {"s", "t", "u", "v", "ss", 
> "tt"} or
>                regtype == "C"        and regid in {"s", "ss"} or
>                regtype == "V"        and regid in {"s", "u", "v", "w", "uu", 
> "vv"}):
>              f.write("//    const int %s = insn->regno[%d];\n" % (regN,
> regno))
>          else:
>              print("Bad register parse: ", regtype, regid)
> 
>      def analyze_opn_new(f, tag, regtype, regid, regno):
>          regN = "%s%sN" % (regtype, regid)
>          if (regtype == "N" and regid in {"s", "t"} or
>              regtype == "P" and regid in {"t", "u", "v"} or
>              regtype == "O" and regid == "s"):
>              f.write("//    const int %s = insn->regno[%d];\n" % (regN,
> regno))
>          else:
>              print("Bad register parse: ", regtype, regid)

That might make sense now, but I wrote it that way to make room in the future 
for further analysis.  For example, I'll mark register reads in addition to 
register writes.  So it will be important to have separate points in the 
if-then-else.

Thanks,
Taylor