> -----Original Message-----
> From: Matheus Tavares Bernardino <quic_mathb...@quicinc.com>
> Sent: Wednesday, July 5, 2023 12:35 PM
> To: qemu-devel@nongnu.org
> Cc: quic_mathb...@quicinc.com; bc...@quicinc.com;
> ltaylorsimp...@gmail.com; quic_mlie...@quicinc.com;
> richard.hender...@linaro.org
> Subject: [PATCH v2] Hexagon: move GETPC() calls to top level helpers
>
> As docs/devel/loads-stores.rst states:
>
> ``GETPC()`` should be used with great care: calling
> it in other functions that are *not* the top level
> ``HELPER(foo)`` will cause unexpected behavior. Instead, the
> value of ``GETPC()`` should be read from the helper and passed
> if needed to the functions that the helper calls.
>
> Let's fix the GETPC() usage in Hexagon, making sure it's always called
from
> top level helpers and passed down to the places where it's needed. There
> are two snippets where that is not currently the case:
>
> - probe_store(), which is only called from two helpers, so it's easy to
> move GETPC() up.
>
> - mem_load*() functions, which are also called directly from helpers,
> but through the MEM_LOAD*() set of macros. Note that this are only
> used when compiling with --disable-hexagon-idef-parser.
>
> In this case, we also take this opportunity to simplify the code,
> unifying the mem_load*() functions.
>
> Signed-off-by: Matheus Tavares Bernardino <quic_mathb...@quicinc.com>
> ---
> v1:
> d40fabcf9d6e92e4cd8d6a144e9b2a9acf4580dc.1688420966.git.quic_mathber
> n...@quicinc.com
>
> Changes in v2:
> - Fixed wrong cpu_ld* unification from previous version.
> - Passed retaddr down to check_noshuf() and further, as Taylor
> suggested.
> - Reorganized macros for simplification.
>
> target/hexagon/macros.h | 19 ++++++------
> target/hexagon/op_helper.h | 11 ++----- target/hexagon/op_helper.c | 62
> +++++++++++---------------------------
> 3 files changed, 29 insertions(+), 63 deletions(-)
>
> diff --git a/target/hexagon/macros.h b/target/hexagon/macros.h index
> 5451b061ee..e44a932434 100644
> --- a/target/hexagon/macros.h
> +++ b/target/hexagon/macros.h
> @@ -173,15 +173,6 @@
> #define fLOAD(NUM, SIZE, SIGN, EA, DST) \
> - DST = (size##SIZE##SIGN##_t)MEM_LOAD##SIZE##SIGN(EA)
> + DST = (size##SIZE##SIGN##_t)({ \
> + check_noshuf(env, pkt_has_store_s1, slot, EA, SIZE, GETPC()); \
> + MEM_LOAD##SIZE(env, EA, GETPC()); \
> + })
> #endif
This should be formatted as
#define fLOAD(...) \
do { \
check_noshuf(...); \
DST = ...; \
} while (0)
> a/target/hexagon/op_helper.h b/target/hexagon/op_helper.h index
> 8f3764d15e..7744e819ef 100644
> --- a/target/hexagon/op_helper.h
> +++ b/target/hexagon/op_helper.h
> +void check_noshuf(CPUHexagonState *env, bool pkt_has_store_s1,
> + uint32_t slot, target_ulong vaddr, int size,
> +uintptr_t ra);
Are you sure this needs to be non-static?
Othersiwe
Reviewed-by: Taylor Simpson <ltaylorsimp...@gmail.com>
