RE: [PATCH v3 3/5] target/hexagon: Add missing A_CALL attr, hintjumpr to multi_cof

[<ltaylorsimpson@gmail.com>]

> -----Original Message-----
> From: Brian Cain <brian.cain@oss.qualcomm.com>
> Sent: Tuesday, April 15, 2025 12:22 PM
> To: ltaylorsimpson@gmail.com; qemu-devel@nongnu.org
> Cc: richard.henderson@linaro.org; philmd@linaro.org;
> matheus.bernardino@oss.qualcomm.com; ale@rev.ng; anjo@rev.ng;
> marco.liebel@oss.qualcomm.com; alex.bennee@linaro.org;
> quic_mburton@quicinc.com; sidneym@quicinc.com
> Subject: Re: [PATCH v3 3/5] target/hexagon: Add missing A_CALL attr,
> hintjumpr to multi_cof
> 
> 
> On 4/14/2025 12:04 PM, ltaylorsimpson@gmail.com wrote:
> >
> >> -----Original Message-----
> >> From: Brian Cain <brian.cain@oss.qualcomm.com>
> >> Sent: Monday, April 7, 2025 1:27 PM
> >> To: qemu-devel@nongnu.org
> >> Cc: brian.cain@oss.qualcomm.com; richard.henderson@linaro.org;
> >> philmd@linaro.org; matheus.bernardino@oss.qualcomm.com;
> ale@rev.ng;
> >> anjo@rev.ng; marco.liebel@oss.qualcomm.com;
> ltaylorsimpson@gmail.com;
> >> alex.bennee@linaro.org; quic_mburton@quicinc.com;
> sidneym@quicinc.com
> >> Subject: [PATCH v3 3/5] target/hexagon: Add missing A_CALL attr,
> >> hintjumpr to multi_cof
> >>
> >> Signed-off-by: Brian Cain <brian.cain@oss.qualcomm.com>
> >> ---
> >>   target/hexagon/hex_common.py | 7 +++++--
> >>   1 file changed, 5 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/target/hexagon/hex_common.py
> >> b/target/hexagon/hex_common.py index 6803908718..a2dcb0aa2e
> 100755
> >> --- a/target/hexagon/hex_common.py
> >> +++ b/target/hexagon/hex_common.py
> >> @@ -247,8 +247,11 @@ def need_next_PC(tag):
> >>
> >>
> >>   def need_pkt_has_multi_cof(tag):
> >> -    return "A_COF" in attribdict[tag]
> >> -
> >> +    return (
> >> +        "A_JUMP" in attribdict[tag]
> >> +        or "A_CALL" in attribdict[tag]
> >> +        or "J2_rte" == tag
> >> +    ) and tag != "J2_hintjumpr"
> > It would be better to make this decision with instruction attributes only
> rather than a mix of attributes and specific tags.  If needed, add another
> add_qemu_macro_attrib call to hex_common.calculate_attribs.
> >
> > Having said that, the correct tag for hintjumpr is J*4*_hintjumpr.
> 
> 
> Good catch, thanks for finding it.  And I suppose we can change it to
> `"A_HINTJR" not in attribdict[tag]` instead.
> 
> 
> So, now more like this:
> 
>       add_qemu_macro_attrib('fREAD_SP', 'A_IMPLICIT_READS_SP')
> +    add_qemu_macro_attrib('fCLEAR_RTE_EX', 'A_RTE')
> 
>       # Recurse down macros, find attributes from sub-macros
>       macroValues = list(macros.values())
> @@ -291,8 +292,8 @@ def need_pkt_has_multi_cof(tag):
>       return (
>           "A_JUMP" in attribdict[tag]
>           or "A_CALL" in attribdict[tag]
> -        or "J2_rte" == tag
> -    ) and tag != "J2_hintjumpr"
> +        or "A_RTE" in attribdict[tag]
> +    ) and "A_HINTJR" not in attribdict[tag]

Let's take a step back here.  The goal is to eliminate the pkt_has_multi_cof parameter from helpers that don't need it, right?

So, the first step is to change a check for A_COF to a check for A_JUMP or A_CALL.  Here are the opcodes where this distinction matters
    J2_endloop*                     These have fGEN_TCG overrides so there is no helper function.
    J2_pause                            Ditto
    J2_rte                                  Ditto
    J4_hintjumpr                     This is a nop in QEMU, and there is an idef-parser emit_J4_hintjumpr function that doesn't generate any TCG.
                                                 When idef-parser is off, you get a helper call, but you could easily override this with an empty fGEN_TCG.
    J2_trap[01]                        These have helper functions, so we want to return false because the helpers don't need this argument.

So the bottom line is you can add an A_TRAP attribute attached to the fTRAP macro and then this function can be
    return "A_COF" in attribdict[tag] and "A_TRAP" not in attribdict[tag]

HTH,
Taylor