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

Brian Cain posted 5 patches 8 months, 2 weeks ago
Maintainers: Brian Cain <brian.cain@oss.qualcomm.com>, Alessandro Di Federico <ale@rev.ng>, Anton Johansson <anjo@rev.ng>
[PATCH v3 3/5] target/hexagon: Add missing A_CALL attr, hintjumpr to multi_cof
Posted by Brian Cain 8 months, 2 weeks ago
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"
 
 def need_pkt_need_commit(tag):
     return 'A_IMPLICIT_WRITES_USR' in attribdict[tag]
-- 
2.34.1

RE: [PATCH v3 3/5] target/hexagon: Add missing A_CALL attr, hintjumpr to multi_cof
Posted by ltaylorsimpson@gmail.com 8 months, 1 week ago

> -----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.

Taylor
Re: [PATCH v3 3/5] target/hexagon: Add missing A_CALL attr, hintjumpr to multi_cof
Posted by Brian Cain 8 months, 1 week ago
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]



> Taylor
>
>

RE: [PATCH v3 3/5] target/hexagon: Add missing A_CALL attr, hintjumpr to multi_cof
Posted by ltaylorsimpson@gmail.com 8 months, 1 week ago

> -----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