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

[Taylor Simpson]

> From: Anton Johansson  
> Sent: Monday, October 18, 2021 6:23 AM
> To: Taylor Simpson ; Alessandro Di Federico 
> ; qemu-devel@nongnu.org
> Cc: Brian Cain ; bab...@rev.ng; ni...@rev.ng; 
> richard.hender...@linaro.org; Alessandro Di Federico 
> Subject: Re: [PATCH v6 07/12] target/hexagon: prepare input for the 
> idef-parser
>
> On 9/7/21 18:09, Taylor Simpson wrote:
> +#define fADDSAT64(DST, A, B)\
> +__a = fCAST8u(A);   \
> +__b = fCAST8u(B);   \
> +__sum = __a + __b;  \
> +__xor = __a ^ __b;  \
> +__mask = fCAST8s(0x8000ULL);\
> +if (((__a ^ __b) | ~(__a ^ __sum)) & __mask) {  \
> +DST = __sum;\
> +} else {\
> +DST = ((__sum & __mask) >> 63) + __mask;\
> +fSET_OVERFLOW();\
> +}
> There are a bunch of these with pretty complex semantics.  Wouldn't it be 
> easier to recognize them in the parser and call a hand-written helper?
> 
> These macro redefinitions are needed to work with the auto type system in 
> idef-parser. We can drop these specializations in the upcoming patchset where 
> we parse fHIDE declarations. 
> If we still want to resort to helpers here, it's probably better to exclude 
> instructions using fADDSAT64 (and similar) directly, and fallback on 
> helpers/overrides for those instructions.

Sorry, I wasn't clear.  I meant a hand-written function to generate the TCG 
code - not a qemu TCG helper.

Thanks,
Taylor

Re: [PATCH v6 07/12] target/hexagon: prepare input for the idef-parser

[Anton Johansson via]

On 9/7/21 18:09, Taylor Simpson wrote:

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

There are a bunch of these with pretty complex semantics.  Wouldn't it be 
easier to recognize them in the parser and call a hand-written helper?

These macro redefinitions are needed to work with the auto type system 
in idef-parser. We can drop these specializations in the upcoming 
patchset where we parse fHIDE declarations.


If we still want to resort to helpers here, it's probably better to 
exclude instructions using fADDSAT64 (and similar) directly, and 
fallback on helpers/overrides for those instructions.


--
Anton Johansson,
rev.ng Srls.

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

[Taylor Simpson]

> -Original Message-
> From: Alessandro Di Federico 
> Sent: Tuesday, July 20, 2021 7:30 AM
> To: qemu-devel@nongnu.org
> Cc: Taylor Simpson ; Brian Cain
> ; bab...@rev.ng; ni...@rev.ng;
> richard.hender...@linaro.org; Alessandro Di Federico 
> Subject: [PATCH v6 07/12] target/hexagon: prepare input for the idef-parser
> 
> From: Alessandro Di Federico 
> 
> Introduce infrastructure necessary to produce a file suitable for being parsed
> by the idef-parser.
> 
> Signed-off-by: Alessandro Di Federico 


> diff --git a/target/hexagon/gen_idef_parser_funcs.py
> b/target/hexagon/gen_idef_parser_funcs.py
> new file mode 100644
> index 00..7b8e0f6981
> --- /dev/null
> +++ b/target/hexagon/gen_idef_parser_funcs.py
> +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])

Do you really need the overrides file?


> diff --git a/target/hexagon/idef-parser/macros.inc b/target/hexagon/idef-
> parser/macros.inc
> new file mode 100644
> index 00..20535691e8
> --- /dev/null
> +++ b/target/hexagon/idef-parser/macros.inc


> +/* Ease parsing */

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

There are a bunch of these with pretty complex semantics.  Wouldn't it be 
easier to recognize them in the parser and call a hand-written helper?

> +/* Assignments */
> +#define fPCALIGN(IMM) (IMM = IMM & ~3)
> +#define fWRITE_LR(A) (LR = A)
> +#define fWRITE_FP(A) (FP = A)
> +#define fWRITE_SP(A) (SP = A)
> +#define fBRANCH(LOC, TYPE) (PC = LOC)
> +#define fJUMPR(REGNO, TARGET, TYPE) (PC = TARGET)

This should invoke write_new_pc because there may be more than one in the same 
packet.

> +fWRITE_NPC(VAL) (PC = VAL)

Ditto

> +/* Binary operators */
> +#define fADD128(A, B) (A + B)
> +#define fSUB128(A, B) (A - B)
> +#define fSHIFTR128(A, B) (size8s_t) (A >> B)
> +#define fSHIFTL128(A, B) (A << B)
> +#define fAND128(A, B) (A & B)

Will these operate on 128 bit numbers?

[PATCH v6 07/12] target/hexagon: prepare input for the idef-parser

[Alessandro Di Federico via]

From: Alessandro Di Federico 

Introduce infrastructure necessary to produce a file suitable for being
parsed by the idef-parser.

Signed-off-by: Alessandro Di Federico 
---
 target/hexagon/gen_idef_parser_funcs.py | 114 ++
 target/hexagon/idef-parser/macros.inc   | 153 
 target/hexagon/idef-parser/prepare  |  24 
 target/hexagon/meson.build  |  16 +++
 4 files changed, 307 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 00..7b8e0f6981
--- /dev/null
+++ b/target/hexagon/gen_idef_parser_funcs.py
@@ -0,0 +1,114 @@
+#!/usr/bin/env python3
+
+##
+##  Copyright(c) 2019-2021 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 .
+##
+
+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