> -----Original Message-----
> From: Brian Cain <brian.c...@oss.qualcomm.com>
> Sent: Friday, February 28, 2025 11:26 PM
> To: qemu-devel@nongnu.org
> Cc: brian.c...@oss.qualcomm.com; richard.hender...@linaro.org;
> phi...@linaro.org; quic_mathb...@quicinc.com; a...@rev.ng; a...@rev.ng;
> quic_mlie...@quicinc.com; ltaylorsimp...@gmail.com;
> alex.ben...@linaro.org; quic_mbur...@quicinc.com;
> sidn...@quicinc.com; Brian Cain <bc...@quicinc.com>
> Subject: [PATCH 21/38] target/hexagon: Add system reg insns
>
> From: Brian Cain <bc...@quicinc.com>
>
> Signed-off-by: Brian Cain <brian.c...@oss.qualcomm.com>
> ---
> target/hexagon/macros.h | 2 +
> target/hexagon/hex_common.py | 15 +-
> target/hexagon/imported/encode_pp.def | 213 +++++++++++++++------
> target/hexagon/imported/system.idef | 262
> +++++++++++++++++++++++---
> 4 files changed, 410 insertions(+), 82 deletions(-)
>
> diff --git a/target/hexagon/macros.h b/target/hexagon/macros.h index
> 6e4a3a1697..b0e9610d98 100644
> --- a/target/hexagon/macros.h
> +++ b/target/hexagon/macros.h
> @@ -675,3 +675,5 @@ static inline TCGv gen_read_ireg(TCGv result, TCGv
> val, int shift) #endif
>
> #endif
> +
> +#define fPREDUSE_TIMING()
Why wasn't this in the previous patch with all the changes to macros.h?
> diff --git a/target/hexagon/hex_common.py
> b/target/hexagon/hex_common.py index 7fb11a0819..9147701333 100755
> --- a/target/hexagon/hex_common.py
> +++ b/target/hexagon/hex_common.py
> @@ -1235,11 +1235,18 @@ def init_registers():
> for reg in new_regs:
> new_registers[f"{reg.regtype}{reg.regid}"] = reg
>
> +def is_new_reg(tag, regid):
> + if regid[0] in "NO":
> + return True
> + return regid[0] == "P" and \
> + f"{regid}N" in semdict[tag] and \
> + f"{regid}V" not in semdict[tag]
> +
> def get_register(tag, regtype, regid):
> - if f"{regtype}{regid}V" in semdict[tag]:
> - return registers[f"{regtype}{regid}"]
> - else:
> - return new_registers[f"{regtype}{regid}"]
> + regid = f"{regtype}{regid}"
> + is_new = is_new_reg(tag, regid)
> + reg = new_registers[regid] if is_new else registers[regid]
> + return reg
This looks OK but is out of place in this patch.
>
> def helper_ret_type(tag, regs):
> ## If there is a scalar result, it is the return type diff --git
> a/target/hexagon/imported/encode_pp.def
> b/target/hexagon/imported/encode_pp.def
> index 0cd30a5e85..37faf62b1b 100644
> --- a/target/hexagon/imported/encode_pp.def
> +++ b/target/hexagon/imported/encode_pp.def
> @@ -1,5 +1,5 @@
> /*
> - * Copyright(c) 2019-2023 Qualcomm Innovation Center, Inc. All Rights
> Reserved.
> + * Copyright(c) 2019-2020 Qualcomm Innovation Center, Inc. All Rights
> Reserved.
Why are you changing the copyright date badkward?
> *
> * 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 @@
> -16,6 +16,7 @@
> */
>
> /*
> + * encode32.def
> * Encodings for 32 bit instructions
> *
> */
> @@ -341,6 +342,8 @@ DEF_ENC32(L4_pload##TAG##tnew_abs,ICLASS_LD"
> 1 11 "OPC" iiiii PP110tti 1--ddd
> DEF_ENC32(L4_pload##TAG##fnew_abs,ICLASS_LD" 1 11 "OPC" iiiii PP111tti
> 1--ddddd")
>
>
> +
> +
> /* 0 000 misc: dealloc,loadw_locked,dcfetch */
> STD_LD_ENC(bzw4,"0 101")
> STD_LD_ENC(bzw2,"0 011")
> @@ -375,6 +378,7 @@ DEF_ANTICLASS32(ICLASS_LD" 1110 000----- PP------ --
> ------",LD_ADDR_POST_REG)
>
> DEF_ENC32(L2_deallocframe, ICLASS_LD" 000 0 000 sssss PP0----- ---
> ddddd")
> DEF_ENC32(L4_return, ICLASS_LD" 011 0 000 sssss PP0000-- ---ddddd")
> +
> DEF_ENC32(L4_return_t, ICLASS_LD" 011 0 000 sssss PP0100vv ---ddddd")
> DEF_ENC32(L4_return_f, ICLASS_LD" 011 0 000 sssss PP1100vv ---ddddd")
> DEF_ENC32(L4_return_tnew_pt, ICLASS_LD" 011 0 000 sssss PP0110vv ---
> ddddd") @@ -382,15 +386,18 @@ DEF_ENC32(L4_return_fnew_pt,
> ICLASS_LD" 011 0 000 sssss PP1110vv ---ddddd")
> DEF_ENC32(L4_return_tnew_pnt, ICLASS_LD" 011 0 000 sssss PP0010vv ---
> ddddd") DEF_ENC32(L4_return_fnew_pnt, ICLASS_LD" 011 0 000 sssss
> PP1010vv ---ddddd")
>
> -DEF_ENC32(L2_loadw_locked,ICLASS_LD" 001 0 000 sssss PP000---
> 000ddddd")
> -
> +/** Load Acquire Store Release Encoding **/
>
> +DEF_ENC32(L2_loadw_locked, ICLASS_LD" 001 0 000 sssss PP000---
> 000ddddd")
> +DEF_ENC32(L4_loadd_locked, ICLASS_LD" 001 0 000 sssss PP010---
> 000ddddd")
There are lots of changes here that look like formatting/whitespace. It would
be easier to review if you could minimize these.
> diff --git a/target/hexagon/imported/system.idef
> b/target/hexagon/imported/system.idef
> index 7c6568e75e..fd7ef18b3e 100644
> --- a/target/hexagon/imported/system.idef
> +++ b/target/hexagon/imported/system.idef
> @@ -25,44 +25,262 @@
> /* User->OS interface */
> /********************************************/
>
> -Q6INSN(J2_trap0,"trap0(#u8)",ATTRIBS(A_COF),
> +Q6INSN(J2_trap0,"trap0(#u8)",ATTRIBS(A_COF,A_NOTE_NOPACKET,A_RES
> TRICT_N
> +OPACKET),
> "Trap to Operating System",
> - fTRAP(0,uiV);
> + fTRAP(0,uiV);
The formatting/whitespace changes are not as rampant in this file, but this is
one example.
> )
