The following changes since commit 36e9aab3c569d4c9ad780473596e18479838d1aa:

migration: Move return path cleanup to main migration thread (2023-09-27 13:58:02 -0400)

https://github.com/quic/qemu tags/pull-hex-20231003

for you to fetch changes up to 53cdd26f1909fc45ad50a5eaa95dcfff06130e5f:

target/hexagon: fix some occurrences of -Wshadow=local (2023-10-03 17:02:29 -0700)

Hexagon GETPC() and -Wshadow fixes

Brian Cain (1):

target/hexagon: fix some occurrences of -Wshadow=local

Matheus Tavares Bernardino (1):

target/hexagon: move GETPC() calls to top level helpers

target/hexagon/imported/alu.idef | 6 +--

target/hexagon/macros.h | 19 +++++----

target/hexagon/mmvec/macros.h | 2 +-

target/hexagon/op_helper.c | 84 +++++++++++++++-------------------------

target/hexagon/op_helper.h | 9 -----

target/hexagon/translate.c | 10 ++---

6 files changed, 50 insertions(+), 80 deletions(-)

Of the changes in this commit, the changes in `HELPER(commit_hvx_stores)()`

are less obvious. They are required because of some macro invocations like

SCATTER_OP_WRITE_TO_MEM().

e.g.:

In file included from ../target/hexagon/op_helper.c:31:

../target/hexagon/mmvec/macros.h:205:18: error: declaration of ‘i’ shadows a previous local [-Werror=shadow=compatible-local]

205 | for (int i = 0; i < sizeof(MMVector); i += sizeof(TYPE)) { \

| ^

../target/hexagon/op_helper.c:157:17: note: in expansion of macro ‘SCATTER_OP_WRITE_TO_MEM’

157 | SCATTER_OP_WRITE_TO_MEM(uint16_t);

| ^~~~~~~~~~~~~~~~~~~~~~~

../target/hexagon/op_helper.c:135:9: note: shadowed declaration is here

135 | int i;

| ^

In file included from ../target/hexagon/op_helper.c:31:

../target/hexagon/mmvec/macros.h:204:19: error: declaration of ‘ra’ shadows a previous local [-Werror=shadow=compatible-local]

204 | uintptr_t ra = GETPC(); \

| ^~

../target/hexagon/op_helper.c:160:17: note: in expansion of macro ‘SCATTER_OP_WRITE_TO_MEM’

160 | SCATTER_OP_WRITE_TO_MEM(uint32_t);

| ^~~~~~~~~~~~~~~~~~~~~~~

../target/hexagon/op_helper.c:134:15: note: shadowed declaration is here

134 | uintptr_t ra = GETPC();

| ^~

Reviewed-by: Matheus Tavares Bernardino <quic_mathbern@quicinc.com>

Signed-off-by: Brian Cain <bcain@quicinc.com>

---

target/hexagon/imported/alu.idef | 6 +++---

target/hexagon/mmvec/macros.h | 2 +-

target/hexagon/op_helper.c | 9 +++------

target/hexagon/translate.c | 10 +++++-----

4 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/target/hexagon/imported/alu.idef b/target/hexagon/imported/alu.idef

index XXXXXXX..XXXXXXX 100644

--- a/target/hexagon/imported/alu.idef

+++ b/target/hexagon/imported/alu.idef

@@ -XXX,XX +XXX,XX @@ Q6INSN(A4_cround_rr,"Rd32=cround(Rs32,Rt32)",ATTRIBS(),"Convergent Round", {RdV

tmp128 = fSHIFTR128(tmp128, SHIFT);\

DST = fCAST16S_8S(tmp128);\

} else {\

- size16s_t rndbit_128 = fCAST8S_16S((1LL << (SHIFT - 1))); \

- size16s_t src_128 = fCAST8S_16S(SRC); \

- size16s_t tmp128 = fADD128(src_128, rndbit_128);\

+ rndbit_128 = fCAST8S_16S((1LL << (SHIFT - 1))); \

+ src_128 = fCAST8S_16S(SRC); \

+ tmp128 = fADD128(src_128, rndbit_128);\

tmp128 = fSHIFTR128(tmp128, SHIFT);\

DST = fCAST16S_8S(tmp128);\

}

diff --git a/target/hexagon/mmvec/macros.h b/target/hexagon/mmvec/macros.h

index XXXXXXX..XXXXXXX 100644

--- a/target/hexagon/mmvec/macros.h

+++ b/target/hexagon/mmvec/macros.h

} while (0)

#define SCATTER_OP_WRITE_TO_MEM(TYPE) \

do { \

- uintptr_t ra = GETPC(); \

+ ra = GETPC(); \

for (int i = 0; i < sizeof(MMVector); i += sizeof(TYPE)) { \

if (test_bit(i, env->vtcm_log.mask)) { \

TYPE dst = 0; \

@@ -XXX,XX +XXX,XX @@ void HELPER(gather_store)(CPUHexagonState *env, uint32_t addr, int slot)

void HELPER(commit_hvx_stores)(CPUHexagonState *env)

{

uintptr_t ra = GETPC();

/* Normal (possibly masked) vector store */

- for (i = 0; i < VSTORES_MAX; i++) {

+ for (int i = 0; i < VSTORES_MAX; i++) {

if (env->vstore_pending[i]) {

env->vstore_pending[i] = 0;

target_ulong va = env->vstore[i].va;

@@ -XXX,XX +XXX,XX @@ void HELPER(commit_hvx_stores)(CPUHexagonState *env)

g_assert_not_reached();

}

} else {

- for (i = 0; i < sizeof(MMVector); i++) {

+ for (int i = 0; i < sizeof(MMVector); i++) {

if (test_bit(i, env->vtcm_log.mask)) {

cpu_stb_data_ra(env, env->vtcm_log.va[i],

env->vtcm_log.data.ub[i], ra);

@@ -XXX,XX +XXX,XX @@ void HELPER(probe_pkt_scalar_store_s0)(CPUHexagonState *env, int args)

static void probe_hvx_stores(CPUHexagonState *env, int mmu_idx,

uintptr_t retaddr)

{

- int i;

-

/* Normal (possibly masked) vector store */

- for (i = 0; i < VSTORES_MAX; i++) {

+ for (int i = 0; i < VSTORES_MAX; i++) {

if (env->vstore_pending[i]) {

target_ulong va = env->vstore[i].va;

int size = env->vstore[i].size;

!bitmap_empty(ctx->predicated_regs, TOTAL_PER_THREAD_REGS)) {

- int i = find_first_bit(ctx->predicated_regs, TOTAL_PER_THREAD_REGS);

+ i = find_first_bit(ctx->predicated_regs, TOTAL_PER_THREAD_REGS);

while (i < TOTAL_PER_THREAD_REGS) {

tcg_gen_mov_tl(get_result_gpr(ctx, i), hex_gpr[i]);

i = find_next_bit(ctx->predicated_regs, TOTAL_PER_THREAD_REGS,

@@ -XXX,XX +XXX,XX @@ static void gen_start_packet(DisasContext *ctx)

* Only endloop instructions conditionally write to pred registers

*/

if (ctx->need_commit && pkt->pkt_has_endloop) {

- for (int i = 0; i < ctx->preg_log_idx; i++) {

+ for (i = 0; i < ctx->preg_log_idx; i++) {

int pred_num = ctx->preg_log[i];

ctx->new_pred_value[pred_num] = tcg_temp_new();

tcg_gen_mov_tl(ctx->new_pred_value[pred_num], hex_pred[pred_num]);

@@ -XXX,XX +XXX,XX @@ static void gen_start_packet(DisasContext *ctx)

/* Preload the predicated HVX registers into future_VRegs and tmp_VRegs */

if (!bitmap_empty(ctx->predicated_future_vregs, NUM_VREGS)) {

- int i = find_first_bit(ctx->predicated_future_vregs, NUM_VREGS);

+ i = find_first_bit(ctx->predicated_future_vregs, NUM_VREGS);

while (i < NUM_VREGS) {

const intptr_t VdV_off =

ctx_future_vreg_off(ctx, i, 1, true);

@@ -XXX,XX +XXX,XX @@ static void gen_start_packet(DisasContext *ctx)

}

}

if (!bitmap_empty(ctx->predicated_tmp_vregs, NUM_VREGS)) {

- int i = find_first_bit(ctx->predicated_tmp_vregs, NUM_VREGS);

+ i = find_first_bit(ctx->predicated_tmp_vregs, NUM_VREGS);

while (i < NUM_VREGS) {

const intptr_t VdV_off =

ctx_tmp_vreg_off(ctx, i, 1, true);

offsetof(CPUHexagonState, mem_log_stores[i].data64),

store_val64_names[i]);

}

- for (int i = 0; i < VSTORES_MAX; i++) {

+ for (i = 0; i < VSTORES_MAX; i++) {

snprintf(vstore_addr_names[i], NAME_LEN, "vstore_addr_%d", i);

hex_vstore_addr[i] = tcg_global_mem_new(cpu_env,

offsetof(CPUHexagonState, vstore[i].va),

2.25.1

From: Matheus Tavares Bernardino <quic_mathbern@quicinc.com>

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

- HELPER(probe_hvx_stores), when called from another helper, ends up

using its own GETPC() expansion instead of the top level caller.

Signed-off-by: Matheus Tavares Bernardino <quic_mathbern@quicinc.com>

Reviewed-by: Taylor Simpson <ltaylorsimpson@gmail.com>

Message-Id: <2c74c3696946edba7cc5b2942cf296a5af532052.1689070412.git.quic_mathbern@quicinc.com>-ne

Reviewed-by: Brian Cain <bcain@quicinc.com>

Signed-off-by: Brian Cain <bcain@quicinc.com>

---

target/hexagon/macros.h | 19 +++++-----

target/hexagon/op_helper.c | 75 +++++++++++++++-----------------------

target/hexagon/op_helper.h | 9 -----

3 files changed, 38 insertions(+), 65 deletions(-)

diff --git a/target/hexagon/macros.h b/target/hexagon/macros.h

--- a/target/hexagon/macros.h

+++ b/target/hexagon/macros.h

@@ -XXX,XX +XXX,XX @@

#define MEM_STORE8(VA, DATA, SLOT) \

MEM_STORE8_FUNC(DATA)(cpu_env, VA, DATA, SLOT)

#else

-#define MEM_LOAD1s(VA) ((int8_t)mem_load1(env, pkt_has_store_s1, slot, VA))

-#define MEM_LOAD1u(VA) ((uint8_t)mem_load1(env, pkt_has_store_s1, slot, VA))

-#define MEM_LOAD2s(VA) ((int16_t)mem_load2(env, pkt_has_store_s1, slot, VA))

-#define MEM_LOAD2u(VA) ((uint16_t)mem_load2(env, pkt_has_store_s1, slot, VA))

-#define MEM_LOAD4s(VA) ((int32_t)mem_load4(env, pkt_has_store_s1, slot, VA))

-#define MEM_LOAD4u(VA) ((uint32_t)mem_load4(env, pkt_has_store_s1, slot, VA))

-#define MEM_LOAD8s(VA) ((int64_t)mem_load8(env, pkt_has_store_s1, slot, VA))

-#define MEM_LOAD8u(VA) ((uint64_t)mem_load8(env, pkt_has_store_s1, slot, VA))

-

#define MEM_STORE1(VA, DATA, SLOT) log_store32(env, VA, DATA, 1, SLOT)

#define MEM_STORE2(VA, DATA, SLOT) log_store32(env, VA, DATA, 2, SLOT)

#define MEM_STORE4(VA, DATA, SLOT) log_store32(env, VA, DATA, 4, SLOT)

@@ -XXX,XX +XXX,XX @@ static inline TCGv gen_read_ireg(TCGv result, TCGv val, int shift)

#ifdef QEMU_GENERATE

#define fLOAD(NUM, SIZE, SIGN, EA, DST) MEM_LOAD##SIZE##SIGN(DST, EA)

#else

+#define MEM_LOAD1 cpu_ldub_data_ra

+#define MEM_LOAD2 cpu_lduw_data_ra

+#define MEM_LOAD4 cpu_ldl_data_ra

+#define MEM_LOAD8 cpu_ldq_data_ra

+

#define fLOAD(NUM, SIZE, SIGN, EA, DST) \

- DST = (size##SIZE##SIGN##_t)MEM_LOAD##SIZE##SIGN(EA)

+ do { \

+ check_noshuf(env, pkt_has_store_s1, slot, EA, SIZE, GETPC()); \

+ DST = (size##SIZE##SIGN##_t)MEM_LOAD##SIZE(env, EA, GETPC()); \

+ } while (0)

#endif

#define fMEMOP(NUM, SIZE, SIGN, EA, FNTYPE, VALUE)

diff --git a/target/hexagon/op_helper.c b/target/hexagon/op_helper.c

--- a/target/hexagon/op_helper.c

+++ b/target/hexagon/op_helper.c

@@ -XXX,XX +XXX,XX @@ void HELPER(debug_check_store_width)(CPUHexagonState *env, int slot, int check)

-void HELPER(commit_store)(CPUHexagonState *env, int slot_num)

+static void commit_store(CPUHexagonState *env, int slot_num, uintptr_t ra)

{

- uintptr_t ra = GETPC();

uint8_t width = env->mem_log_stores[slot_num].width;

target_ulong va = env->mem_log_stores[slot_num].va;

@@ -XXX,XX +XXX,XX @@ void HELPER(commit_store)(CPUHexagonState *env, int slot_num)

+void HELPER(commit_store)(CPUHexagonState *env, int slot_num)

+{

+ uintptr_t ra = GETPC();

+ commit_store(env, slot_num, ra);

+}

+

void HELPER(gather_store)(CPUHexagonState *env, uint32_t addr, int slot)

{

mem_gather_store(env, addr, slot);

@@ -XXX,XX +XXX,XX @@ int32_t HELPER(cabacdecbin_pred)(int64_t RssV, int64_t RttV)

}

static void probe_store(CPUHexagonState *env, int slot, int mmu_idx,

- bool is_predicated)

+ bool is_predicated, uintptr_t retaddr)

{

if (!is_predicated || !(env->slot_cancelled & (1 << slot))) {

size1u_t width = env->mem_log_stores[slot].width;

target_ulong va = env->mem_log_stores[slot].va;

- uintptr_t ra = GETPC();

- probe_write(env, va, width, mmu_idx, ra);

+ probe_write(env, va, width, mmu_idx, retaddr);

2.25.1