[PATCH 0/5] target/mips: Replace TARGET_WORDS_BIGENDIAN by cpu_is_bigendian()

[PATCH 0/5] target/mips: Replace TARGET_WORDS_BIGENDIAN by cpu_is_bigendian()

[Philippe Mathieu-Daudé]

MIPS CPU store its endianess in the CP0 Config0 register.
Use that runtime information instead of #ifdef'ry checking
TARGET_WORDS_BIGENDIAN by introducing the cpu_is_bigendian()
helper.

Philippe Mathieu-Daudé (5):
  target/mips: Replace GET_OFFSET() macro by get_offset() function
  target/mips: Replace GET_LMASK() macro by get_lmask(32) function
  target/mips: Replace GET_LMASK64() macro by get_lmask(64) function
  target/mips: Store CP0_Config0 in DisasContext
  target/mips: Replace TARGET_WORDS_BIGENDIAN by cpu_is_bigendian()

 target/mips/tcg/translate.h              |   6 ++
 target/mips/tcg/ldst_helper.c            | 120 +++++++++++++----------
 target/mips/tcg/translate.c              |  71 +++++++-------
 target/mips/tcg/nanomips_translate.c.inc |  20 ++--
 4 files changed, 120 insertions(+), 97 deletions(-)

-- 
2.31.1

[PATCH 1/5] target/mips: Replace GET_OFFSET() macro by get_offset() function

[Philippe Mathieu-Daudé]

The target endianess information is stored in the BigEndian
bit of the Config0 register in CP0.

As a first step, replace the GET_OFFSET() macro by an inlined
get_offset() function, passing CPUMIPSState as argument.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/mips/tcg/ldst_helper.c | 57 +++++++++++++++++++++--------------
 1 file changed, 35 insertions(+), 22 deletions(-)

diff --git a/target/mips/tcg/ldst_helper.c b/target/mips/tcg/ldst_helper.c
index d42812b8a6a..97e7ad7d7a4 100644
--- a/target/mips/tcg/ldst_helper.c
+++ b/target/mips/tcg/ldst_helper.c
@@ -52,31 +52,44 @@ HELPER_LD_ATOMIC(lld, ldq, 0x7, (target_ulong))
 
 #endif /* !CONFIG_USER_ONLY */
 
+static inline bool cpu_is_bigendian(CPUMIPSState *env)
+{
+    return extract32(env->CP0_Config0, CP0C0_BE, 1);
+}
+
 #ifdef TARGET_WORDS_BIGENDIAN
 #define GET_LMASK(v) ((v) & 3)
-#define GET_OFFSET(addr, offset) (addr + (offset))
 #else
 #define GET_LMASK(v) (((v) & 3) ^ 3)
-#define GET_OFFSET(addr, offset) (addr - (offset))
 #endif
 
+static inline target_ulong get_offset(CPUMIPSState *env,
+                                      target_ulong addr, int offset)
+{
+    if (cpu_is_bigendian(env)) {
+        return addr + offset;
+    } else {
+        return addr - offset;
+    }
+}
+
 void helper_swl(CPUMIPSState *env, target_ulong arg1, target_ulong arg2,
                 int mem_idx)
 {
     cpu_stb_mmuidx_ra(env, arg2, (uint8_t)(arg1 >> 24), mem_idx, GETPC());
 
     if (GET_LMASK(arg2) <= 2) {
-        cpu_stb_mmuidx_ra(env, GET_OFFSET(arg2, 1), (uint8_t)(arg1 >> 16),
+        cpu_stb_mmuidx_ra(env, get_offset(env, arg2, 1), (uint8_t)(arg1 >> 16),
                           mem_idx, GETPC());
     }
 
     if (GET_LMASK(arg2) <= 1) {
-        cpu_stb_mmuidx_ra(env, GET_OFFSET(arg2, 2), (uint8_t)(arg1 >> 8),
+        cpu_stb_mmuidx_ra(env, get_offset(env, arg2, 2), (uint8_t)(arg1 >> 8),
                           mem_idx, GETPC());
     }
 
     if (GET_LMASK(arg2) == 0) {
-        cpu_stb_mmuidx_ra(env, GET_OFFSET(arg2, 3), (uint8_t)arg1,
+        cpu_stb_mmuidx_ra(env, get_offset(env, arg2, 3), (uint8_t)arg1,
                           mem_idx, GETPC());
     }
 }
@@ -87,17 +100,17 @@ void helper_swr(CPUMIPSState *env, target_ulong arg1, target_ulong arg2,
     cpu_stb_mmuidx_ra(env, arg2, (uint8_t)arg1, mem_idx, GETPC());
 
     if (GET_LMASK(arg2) >= 1) {
-        cpu_stb_mmuidx_ra(env, GET_OFFSET(arg2, -1), (uint8_t)(arg1 >> 8),
+        cpu_stb_mmuidx_ra(env, get_offset(env, arg2, -1), (uint8_t)(arg1 >> 8),
                           mem_idx, GETPC());
     }
 
     if (GET_LMASK(arg2) >= 2) {
-        cpu_stb_mmuidx_ra(env, GET_OFFSET(arg2, -2), (uint8_t)(arg1 >> 16),
+        cpu_stb_mmuidx_ra(env, get_offset(env, arg2, -2), (uint8_t)(arg1 >> 16),
                           mem_idx, GETPC());
     }
 
     if (GET_LMASK(arg2) == 3) {
-        cpu_stb_mmuidx_ra(env, GET_OFFSET(arg2, -3), (uint8_t)(arg1 >> 24),
+        cpu_stb_mmuidx_ra(env, get_offset(env, arg2, -3), (uint8_t)(arg1 >> 24),
                           mem_idx, GETPC());
     }
 }
@@ -119,37 +132,37 @@ void helper_sdl(CPUMIPSState *env, target_ulong arg1, target_ulong arg2,
     cpu_stb_mmuidx_ra(env, arg2, (uint8_t)(arg1 >> 56), mem_idx, GETPC());
 
     if (GET_LMASK64(arg2) <= 6) {
-        cpu_stb_mmuidx_ra(env, GET_OFFSET(arg2, 1), (uint8_t)(arg1 >> 48),
+        cpu_stb_mmuidx_ra(env, get_offset(env, arg2, 1), (uint8_t)(arg1 >> 48),
                           mem_idx, GETPC());
     }
 
     if (GET_LMASK64(arg2) <= 5) {
-        cpu_stb_mmuidx_ra(env, GET_OFFSET(arg2, 2), (uint8_t)(arg1 >> 40),
+        cpu_stb_mmuidx_ra(env, get_offset(env, arg2, 2), (uint8_t)(arg1 >> 40),
                           mem_idx, GETPC());
     }
 
     if (GET_LMASK64(arg2) <= 4) {
-        cpu_stb_mmuidx_ra(env, GET_OFFSET(arg2, 3), (uint8_t)(arg1 >> 32),
+        cpu_stb_mmuidx_ra(env, get_offset(env, arg2, 3), (uint8_t)(arg1 >> 32),
                           mem_idx, GETPC());
     }
 
     if (GET_LMASK64(arg2) <= 3) {
-        cpu_stb_mmuidx_ra(env, GET_OFFSET(arg2, 4), (uint8_t)(arg1 >> 24),
+        cpu_stb_mmuidx_ra(env, get_offset(env, arg2, 4), (uint8_t)(arg1 >> 24),
                           mem_idx, GETPC());
     }
 
     if (GET_LMASK64(arg2) <= 2) {
-        cpu_stb_mmuidx_ra(env, GET_OFFSET(arg2, 5), (uint8_t)(arg1 >> 16),
+        cpu_stb_mmuidx_ra(env, get_offset(env, arg2, 5), (uint8_t)(arg1 >> 16),
                           mem_idx, GETPC());
     }
 
     if (GET_LMASK64(arg2) <= 1) {
-        cpu_stb_mmuidx_ra(env, GET_OFFSET(arg2, 6), (uint8_t)(arg1 >> 8),
+        cpu_stb_mmuidx_ra(env, get_offset(env, arg2, 6), (uint8_t)(arg1 >> 8),
                           mem_idx, GETPC());
     }
 
     if (GET_LMASK64(arg2) <= 0) {
-        cpu_stb_mmuidx_ra(env, GET_OFFSET(arg2, 7), (uint8_t)arg1,
+        cpu_stb_mmuidx_ra(env, get_offset(env, arg2, 7), (uint8_t)arg1,
                           mem_idx, GETPC());
     }
 }
@@ -160,37 +173,37 @@ void helper_sdr(CPUMIPSState *env, target_ulong arg1, target_ulong arg2,
     cpu_stb_mmuidx_ra(env, arg2, (uint8_t)arg1, mem_idx, GETPC());
 
     if (GET_LMASK64(arg2) >= 1) {
-        cpu_stb_mmuidx_ra(env, GET_OFFSET(arg2, -1), (uint8_t)(arg1 >> 8),
+        cpu_stb_mmuidx_ra(env, get_offset(env, arg2, -1), (uint8_t)(arg1 >> 8),
                           mem_idx, GETPC());
     }
 
     if (GET_LMASK64(arg2) >= 2) {
-        cpu_stb_mmuidx_ra(env, GET_OFFSET(arg2, -2), (uint8_t)(arg1 >> 16),
+        cpu_stb_mmuidx_ra(env, get_offset(env, arg2, -2), (uint8_t)(arg1 >> 16),
                           mem_idx, GETPC());
     }
 
     if (GET_LMASK64(arg2) >= 3) {
-        cpu_stb_mmuidx_ra(env, GET_OFFSET(arg2, -3), (uint8_t)(arg1 >> 24),
+        cpu_stb_mmuidx_ra(env, get_offset(env, arg2, -3), (uint8_t)(arg1 >> 24),
                           mem_idx, GETPC());
     }
 
     if (GET_LMASK64(arg2) >= 4) {
-        cpu_stb_mmuidx_ra(env, GET_OFFSET(arg2, -4), (uint8_t)(arg1 >> 32),
+        cpu_stb_mmuidx_ra(env, get_offset(env, arg2, -4), (uint8_t)(arg1 >> 32),
                           mem_idx, GETPC());
     }
 
     if (GET_LMASK64(arg2) >= 5) {
-        cpu_stb_mmuidx_ra(env, GET_OFFSET(arg2, -5), (uint8_t)(arg1 >> 40),
+        cpu_stb_mmuidx_ra(env, get_offset(env, arg2, -5), (uint8_t)(arg1 >> 40),
                           mem_idx, GETPC());
     }
 
     if (GET_LMASK64(arg2) >= 6) {
-        cpu_stb_mmuidx_ra(env, GET_OFFSET(arg2, -6), (uint8_t)(arg1 >> 48),
+        cpu_stb_mmuidx_ra(env, get_offset(env, arg2, -6), (uint8_t)(arg1 >> 48),
                           mem_idx, GETPC());
     }
 
     if (GET_LMASK64(arg2) == 7) {
-        cpu_stb_mmuidx_ra(env, GET_OFFSET(arg2, -7), (uint8_t)(arg1 >> 56),
+        cpu_stb_mmuidx_ra(env, get_offset(env, arg2, -7), (uint8_t)(arg1 >> 56),
                           mem_idx, GETPC());
     }
 }
-- 
2.31.1

[PATCH 2/5] target/mips: Replace GET_LMASK() macro by get_lmask(32) function

[Philippe Mathieu-Daudé]

The target endianess information is stored in the BigEndian
bit of the Config0 register in CP0.

Replace the GET_LMASK() macro by an inlined get_lmask() function,
passing CPUMIPSState and the word size as argument.

We can remove one use of the TARGET_WORDS_BIGENDIAN definition.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/mips/tcg/ldst_helper.c | 32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/target/mips/tcg/ldst_helper.c b/target/mips/tcg/ldst_helper.c
index 97e7ad7d7a4..888578c0b9c 100644
--- a/target/mips/tcg/ldst_helper.c
+++ b/target/mips/tcg/ldst_helper.c
@@ -57,12 +57,6 @@ static inline bool cpu_is_bigendian(CPUMIPSState *env)
     return extract32(env->CP0_Config0, CP0C0_BE, 1);
 }
 
-#ifdef TARGET_WORDS_BIGENDIAN
-#define GET_LMASK(v) ((v) & 3)
-#else
-#define GET_LMASK(v) (((v) & 3) ^ 3)
-#endif
-
 static inline target_ulong get_offset(CPUMIPSState *env,
                                       target_ulong addr, int offset)
 {
@@ -73,22 +67,36 @@ static inline target_ulong get_offset(CPUMIPSState *env,
     }
 }
 
+static inline target_ulong get_lmask(CPUMIPSState *env,
+                                     target_ulong value, unsigned bits)
+{
+    unsigned mask = (bits / BITS_PER_BYTE) - 1;
+
+    value &= mask;
+
+    if (cpu_is_bigendian(env)) {
+        value ^= mask;
+    }
+
+    return value;
+}
+
 void helper_swl(CPUMIPSState *env, target_ulong arg1, target_ulong arg2,
                 int mem_idx)
 {
     cpu_stb_mmuidx_ra(env, arg2, (uint8_t)(arg1 >> 24), mem_idx, GETPC());
 
-    if (GET_LMASK(arg2) <= 2) {
+    if (get_lmask(env, arg2, 32) <= 2) {
         cpu_stb_mmuidx_ra(env, get_offset(env, arg2, 1), (uint8_t)(arg1 >> 16),
                           mem_idx, GETPC());
     }
 
-    if (GET_LMASK(arg2) <= 1) {
+    if (get_lmask(env, arg2, 32) <= 1) {
         cpu_stb_mmuidx_ra(env, get_offset(env, arg2, 2), (uint8_t)(arg1 >> 8),
                           mem_idx, GETPC());
     }
 
-    if (GET_LMASK(arg2) == 0) {
+    if (get_lmask(env, arg2, 32) == 0) {
         cpu_stb_mmuidx_ra(env, get_offset(env, arg2, 3), (uint8_t)arg1,
                           mem_idx, GETPC());
     }
@@ -99,17 +107,17 @@ void helper_swr(CPUMIPSState *env, target_ulong arg1, target_ulong arg2,
 {
     cpu_stb_mmuidx_ra(env, arg2, (uint8_t)arg1, mem_idx, GETPC());
 
-    if (GET_LMASK(arg2) >= 1) {
+    if (get_lmask(env, arg2, 32) >= 1) {
         cpu_stb_mmuidx_ra(env, get_offset(env, arg2, -1), (uint8_t)(arg1 >> 8),
                           mem_idx, GETPC());
     }
 
-    if (GET_LMASK(arg2) >= 2) {
+    if (get_lmask(env, arg2, 32) >= 2) {
         cpu_stb_mmuidx_ra(env, get_offset(env, arg2, -2), (uint8_t)(arg1 >> 16),
                           mem_idx, GETPC());
     }
 
-    if (GET_LMASK(arg2) == 3) {
+    if (get_lmask(env, arg2, 32) == 3) {
         cpu_stb_mmuidx_ra(env, get_offset(env, arg2, -3), (uint8_t)(arg1 >> 24),
                           mem_idx, GETPC());
     }
-- 
2.31.1

[PATCH 3/5] target/mips: Replace GET_LMASK64() macro by get_lmask(64) function

[Philippe Mathieu-Daudé]

The target endianess information is stored in the BigEndian
bit of the Config0 register in CP0.

Replace the GET_LMASK() macro by an inlined get_lmask() function,
passing CPUMIPSState and the word size as argument.

We can remove another use of the TARGET_WORDS_BIGENDIAN definition.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/mips/tcg/ldst_helper.c | 33 ++++++++++++++-------------------
 1 file changed, 14 insertions(+), 19 deletions(-)

diff --git a/target/mips/tcg/ldst_helper.c b/target/mips/tcg/ldst_helper.c
index 888578c0b9c..01b3dc5bee0 100644
--- a/target/mips/tcg/ldst_helper.c
+++ b/target/mips/tcg/ldst_helper.c
@@ -128,48 +128,43 @@ void helper_swr(CPUMIPSState *env, target_ulong arg1, target_ulong arg2,
  * "half" load and stores.  We must do the memory access inline,
  * or fault handling won't work.
  */
-#ifdef TARGET_WORDS_BIGENDIAN
-#define GET_LMASK64(v) ((v) & 7)
-#else
-#define GET_LMASK64(v) (((v) & 7) ^ 7)
-#endif
 
 void helper_sdl(CPUMIPSState *env, target_ulong arg1, target_ulong arg2,
                 int mem_idx)
 {
     cpu_stb_mmuidx_ra(env, arg2, (uint8_t)(arg1 >> 56), mem_idx, GETPC());
 
-    if (GET_LMASK64(arg2) <= 6) {
+    if (get_lmask(env, arg2, 64) <= 6) {
         cpu_stb_mmuidx_ra(env, get_offset(env, arg2, 1), (uint8_t)(arg1 >> 48),
                           mem_idx, GETPC());
     }
 
-    if (GET_LMASK64(arg2) <= 5) {
+    if (get_lmask(env, arg2, 64) <= 5) {
         cpu_stb_mmuidx_ra(env, get_offset(env, arg2, 2), (uint8_t)(arg1 >> 40),
                           mem_idx, GETPC());
     }
 
-    if (GET_LMASK64(arg2) <= 4) {
+    if (get_lmask(env, arg2, 64) <= 4) {
         cpu_stb_mmuidx_ra(env, get_offset(env, arg2, 3), (uint8_t)(arg1 >> 32),
                           mem_idx, GETPC());
     }
 
-    if (GET_LMASK64(arg2) <= 3) {
+    if (get_lmask(env, arg2, 64) <= 3) {
         cpu_stb_mmuidx_ra(env, get_offset(env, arg2, 4), (uint8_t)(arg1 >> 24),
                           mem_idx, GETPC());
     }
 
-    if (GET_LMASK64(arg2) <= 2) {
+    if (get_lmask(env, arg2, 64) <= 2) {
         cpu_stb_mmuidx_ra(env, get_offset(env, arg2, 5), (uint8_t)(arg1 >> 16),
                           mem_idx, GETPC());
     }
 
-    if (GET_LMASK64(arg2) <= 1) {
+    if (get_lmask(env, arg2, 64) <= 1) {
         cpu_stb_mmuidx_ra(env, get_offset(env, arg2, 6), (uint8_t)(arg1 >> 8),
                           mem_idx, GETPC());
     }
 
-    if (GET_LMASK64(arg2) <= 0) {
+    if (get_lmask(env, arg2, 64) <= 0) {
         cpu_stb_mmuidx_ra(env, get_offset(env, arg2, 7), (uint8_t)arg1,
                           mem_idx, GETPC());
     }
@@ -180,37 +175,37 @@ void helper_sdr(CPUMIPSState *env, target_ulong arg1, target_ulong arg2,
 {
     cpu_stb_mmuidx_ra(env, arg2, (uint8_t)arg1, mem_idx, GETPC());
 
-    if (GET_LMASK64(arg2) >= 1) {
+    if (get_lmask(env, arg2, 64) >= 1) {
         cpu_stb_mmuidx_ra(env, get_offset(env, arg2, -1), (uint8_t)(arg1 >> 8),
                           mem_idx, GETPC());
     }
 
-    if (GET_LMASK64(arg2) >= 2) {
+    if (get_lmask(env, arg2, 64) >= 2) {
         cpu_stb_mmuidx_ra(env, get_offset(env, arg2, -2), (uint8_t)(arg1 >> 16),
                           mem_idx, GETPC());
     }
 
-    if (GET_LMASK64(arg2) >= 3) {
+    if (get_lmask(env, arg2, 64) >= 3) {
         cpu_stb_mmuidx_ra(env, get_offset(env, arg2, -3), (uint8_t)(arg1 >> 24),
                           mem_idx, GETPC());
     }
 
-    if (GET_LMASK64(arg2) >= 4) {
+    if (get_lmask(env, arg2, 64) >= 4) {
         cpu_stb_mmuidx_ra(env, get_offset(env, arg2, -4), (uint8_t)(arg1 >> 32),
                           mem_idx, GETPC());
     }
 
-    if (GET_LMASK64(arg2) >= 5) {
+    if (get_lmask(env, arg2, 64) >= 5) {
         cpu_stb_mmuidx_ra(env, get_offset(env, arg2, -5), (uint8_t)(arg1 >> 40),
                           mem_idx, GETPC());
     }
 
-    if (GET_LMASK64(arg2) >= 6) {
+    if (get_lmask(env, arg2, 64) >= 6) {
         cpu_stb_mmuidx_ra(env, get_offset(env, arg2, -6), (uint8_t)(arg1 >> 48),
                           mem_idx, GETPC());
     }
 
-    if (GET_LMASK64(arg2) == 7) {
+    if (get_lmask(env, arg2, 64) == 7) {
         cpu_stb_mmuidx_ra(env, get_offset(env, arg2, -7), (uint8_t)(arg1 >> 56),
                           mem_idx, GETPC());
     }
-- 
2.31.1

[PATCH 4/5] target/mips: Store CP0_Config0 in DisasContext

[Philippe Mathieu-Daudé]

Most TCG helpers only have access to a DisasContext pointer,
not CPUMIPSState. Store a copy of CPUMIPSState::CP0_Config0
in DisasContext so we can access it from TCG helpers.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/mips/tcg/translate.h | 1 +
 target/mips/tcg/translate.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/target/mips/tcg/translate.h b/target/mips/tcg/translate.h
index bb0a6b8d74f..9d325c836aa 100644
--- a/target/mips/tcg/translate.h
+++ b/target/mips/tcg/translate.h
@@ -18,6 +18,7 @@ typedef struct DisasContext {
     target_ulong page_start;
     uint32_t opcode;
     uint64_t insn_flags;
+    int32_t CP0_Config0;
     int32_t CP0_Config1;
     int32_t CP0_Config2;
     int32_t CP0_Config3;
diff --git a/target/mips/tcg/translate.c b/target/mips/tcg/translate.c
index a58d50e40e2..572104e2cc2 100644
--- a/target/mips/tcg/translate.c
+++ b/target/mips/tcg/translate.c
@@ -16034,6 +16034,7 @@ static void mips_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
     ctx->page_start = ctx->base.pc_first & TARGET_PAGE_MASK;
     ctx->saved_pc = -1;
     ctx->insn_flags = env->insn_flags;
+    ctx->CP0_Config0 = env->CP0_Config0;
     ctx->CP0_Config1 = env->CP0_Config1;
     ctx->CP0_Config2 = env->CP0_Config2;
     ctx->CP0_Config3 = env->CP0_Config3;
-- 
2.31.1

[PATCH 5/5] target/mips: Replace TARGET_WORDS_BIGENDIAN by cpu_is_bigendian()

[Philippe Mathieu-Daudé]

Add the inlined cpu_is_bigendian() function in "translate.h".

Replace the TARGET_WORDS_BIGENDIAN #ifdef'ry by calls to
cpu_is_bigendian().

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/mips/tcg/translate.h              |  5 ++
 target/mips/tcg/translate.c              | 70 ++++++++++++------------
 target/mips/tcg/nanomips_translate.c.inc | 20 +++----
 3 files changed, 50 insertions(+), 45 deletions(-)

diff --git a/target/mips/tcg/translate.h b/target/mips/tcg/translate.h
index 9d325c836aa..dfb1552c2fc 100644
--- a/target/mips/tcg/translate.h
+++ b/target/mips/tcg/translate.h
@@ -212,4 +212,9 @@ bool decode_ext_vr54xx(DisasContext *ctx, uint32_t insn);
     static bool trans_##NAME(DisasContext *ctx, arg_##NAME *a) \
     { return FUNC(ctx, a, __VA_ARGS__); }
 
+static inline bool cpu_is_bigendian(DisasContext *ctx)
+{
+    return extract32(ctx->CP0_Config0, CP0C0_BE, 1);
+}
+
 #endif
diff --git a/target/mips/tcg/translate.c b/target/mips/tcg/translate.c
index 572104e2cc2..f182e64643d 100644
--- a/target/mips/tcg/translate.c
+++ b/target/mips/tcg/translate.c
@@ -2093,9 +2093,9 @@ static void gen_ld(DisasContext *ctx, uint32_t opc,
          */
         tcg_gen_qemu_ld_tl(t1, t0, mem_idx, MO_UB);
         tcg_gen_andi_tl(t1, t0, 7);
-#ifndef TARGET_WORDS_BIGENDIAN
-        tcg_gen_xori_tl(t1, t1, 7);
-#endif
+        if (!cpu_is_bigendian(ctx)) {
+            tcg_gen_xori_tl(t1, t1, 7);
+        }
         tcg_gen_shli_tl(t1, t1, 3);
         tcg_gen_andi_tl(t0, t0, ~7);
         tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_TEQ);
@@ -2117,9 +2117,9 @@ static void gen_ld(DisasContext *ctx, uint32_t opc,
          */
         tcg_gen_qemu_ld_tl(t1, t0, mem_idx, MO_UB);
         tcg_gen_andi_tl(t1, t0, 7);
-#ifdef TARGET_WORDS_BIGENDIAN
-        tcg_gen_xori_tl(t1, t1, 7);
-#endif
+        if (cpu_is_bigendian(ctx)) {
+            tcg_gen_xori_tl(t1, t1, 7);
+        }
         tcg_gen_shli_tl(t1, t1, 3);
         tcg_gen_andi_tl(t0, t0, ~7);
         tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_TEQ);
@@ -2198,9 +2198,9 @@ static void gen_ld(DisasContext *ctx, uint32_t opc,
          */
         tcg_gen_qemu_ld_tl(t1, t0, mem_idx, MO_UB);
         tcg_gen_andi_tl(t1, t0, 3);
-#ifndef TARGET_WORDS_BIGENDIAN
-        tcg_gen_xori_tl(t1, t1, 3);
-#endif
+        if (!cpu_is_bigendian(ctx)) {
+            tcg_gen_xori_tl(t1, t1, 3);
+        }
         tcg_gen_shli_tl(t1, t1, 3);
         tcg_gen_andi_tl(t0, t0, ~3);
         tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_TEUL);
@@ -2226,9 +2226,9 @@ static void gen_ld(DisasContext *ctx, uint32_t opc,
          */
         tcg_gen_qemu_ld_tl(t1, t0, mem_idx, MO_UB);
         tcg_gen_andi_tl(t1, t0, 3);
-#ifdef TARGET_WORDS_BIGENDIAN
-        tcg_gen_xori_tl(t1, t1, 3);
-#endif
+        if (cpu_is_bigendian(ctx)) {
+            tcg_gen_xori_tl(t1, t1, 3);
+        }
         tcg_gen_shli_tl(t1, t1, 3);
         tcg_gen_andi_tl(t0, t0, ~3);
         tcg_gen_qemu_ld_tl(t0, t0, mem_idx, MO_TEUL);
@@ -4445,9 +4445,9 @@ static void gen_loongson_lswc2(DisasContext *ctx, int rt,
             t1 = tcg_temp_new();
             tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx, MO_UB);
             tcg_gen_andi_tl(t1, t0, 3);
-#ifndef TARGET_WORDS_BIGENDIAN
-            tcg_gen_xori_tl(t1, t1, 3);
-#endif
+            if (!cpu_is_bigendian(ctx)) {
+                tcg_gen_xori_tl(t1, t1, 3);
+            }
             tcg_gen_shli_tl(t1, t1, 3);
             tcg_gen_andi_tl(t0, t0, ~3);
             tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TEUL);
@@ -4475,9 +4475,9 @@ static void gen_loongson_lswc2(DisasContext *ctx, int rt,
             t1 = tcg_temp_new();
             tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx, MO_UB);
             tcg_gen_andi_tl(t1, t0, 3);
-#ifdef TARGET_WORDS_BIGENDIAN
-            tcg_gen_xori_tl(t1, t1, 3);
-#endif
+            if (cpu_is_bigendian(ctx)) {
+                tcg_gen_xori_tl(t1, t1, 3);
+            }
             tcg_gen_shli_tl(t1, t1, 3);
             tcg_gen_andi_tl(t0, t0, ~3);
             tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TEUL);
@@ -4507,9 +4507,9 @@ static void gen_loongson_lswc2(DisasContext *ctx, int rt,
             t1 = tcg_temp_new();
             tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx, MO_UB);
             tcg_gen_andi_tl(t1, t0, 7);
-#ifndef TARGET_WORDS_BIGENDIAN
-            tcg_gen_xori_tl(t1, t1, 7);
-#endif
+            if (!cpu_is_bigendian(ctx)) {
+                tcg_gen_xori_tl(t1, t1, 7);
+            }
             tcg_gen_shli_tl(t1, t1, 3);
             tcg_gen_andi_tl(t0, t0, ~7);
             tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TEQ);
@@ -4529,9 +4529,9 @@ static void gen_loongson_lswc2(DisasContext *ctx, int rt,
             t1 = tcg_temp_new();
             tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx, MO_UB);
             tcg_gen_andi_tl(t1, t0, 7);
-#ifdef TARGET_WORDS_BIGENDIAN
-            tcg_gen_xori_tl(t1, t1, 7);
-#endif
+            if (cpu_is_bigendian(ctx)) {
+                tcg_gen_xori_tl(t1, t1, 7);
+            }
             tcg_gen_shli_tl(t1, t1, 3);
             tcg_gen_andi_tl(t0, t0, ~7);
             tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TEQ);
@@ -11464,17 +11464,17 @@ static void gen_flt3_arith(DisasContext *ctx, uint32_t opc,
             gen_set_label(l1);
             tcg_gen_brcondi_tl(TCG_COND_NE, t0, 4, l2);
             tcg_temp_free(t0);
-#ifdef TARGET_WORDS_BIGENDIAN
-            gen_load_fpr32(ctx, fp, fs);
-            gen_load_fpr32h(ctx, fph, ft);
-            gen_store_fpr32h(ctx, fp, fd);
-            gen_store_fpr32(ctx, fph, fd);
-#else
-            gen_load_fpr32h(ctx, fph, fs);
-            gen_load_fpr32(ctx, fp, ft);
-            gen_store_fpr32(ctx, fph, fd);
-            gen_store_fpr32h(ctx, fp, fd);
-#endif
+            if (cpu_is_bigendian(ctx)) {
+                gen_load_fpr32(ctx, fp, fs);
+                gen_load_fpr32h(ctx, fph, ft);
+                gen_store_fpr32h(ctx, fp, fd);
+                gen_store_fpr32(ctx, fph, fd);
+            } else {
+                gen_load_fpr32h(ctx, fph, fs);
+                gen_load_fpr32(ctx, fp, ft);
+                gen_store_fpr32(ctx, fph, fd);
+                gen_store_fpr32h(ctx, fp, fd);
+            }
             gen_set_label(l2);
             tcg_temp_free_i32(fp);
             tcg_temp_free_i32(fph);
diff --git a/target/mips/tcg/nanomips_translate.c.inc b/target/mips/tcg/nanomips_translate.c.inc
index 09e64a69480..a66ae267963 100644
--- a/target/mips/tcg/nanomips_translate.c.inc
+++ b/target/mips/tcg/nanomips_translate.c.inc
@@ -999,11 +999,11 @@ static void gen_llwp(DisasContext *ctx, uint32_t base, int16_t offset,
 
     gen_base_offset_addr(ctx, taddr, base, offset);
     tcg_gen_qemu_ld64(tval, taddr, ctx->mem_idx);
-#ifdef TARGET_WORDS_BIGENDIAN
-    tcg_gen_extr_i64_tl(tmp2, tmp1, tval);
-#else
-    tcg_gen_extr_i64_tl(tmp1, tmp2, tval);
-#endif
+    if (cpu_is_bigendian(ctx)) {
+        tcg_gen_extr_i64_tl(tmp2, tmp1, tval);
+    } else {
+        tcg_gen_extr_i64_tl(tmp1, tmp2, tval);
+    }
     gen_store_gpr(tmp1, reg1);
     tcg_temp_free(tmp1);
     gen_store_gpr(tmp2, reg2);
@@ -1035,11 +1035,11 @@ static void gen_scwp(DisasContext *ctx, uint32_t base, int16_t offset,
     gen_load_gpr(tmp1, reg1);
     gen_load_gpr(tmp2, reg2);
 
-#ifdef TARGET_WORDS_BIGENDIAN
-    tcg_gen_concat_tl_i64(tval, tmp2, tmp1);
-#else
-    tcg_gen_concat_tl_i64(tval, tmp1, tmp2);
-#endif
+    if (cpu_is_bigendian(ctx)) {
+        tcg_gen_concat_tl_i64(tval, tmp2, tmp1);
+    } else {
+        tcg_gen_concat_tl_i64(tval, tmp1, tmp2);
+    }
 
     tcg_gen_ld_i64(llval, cpu_env, offsetof(CPUMIPSState, llval_wp));
     tcg_gen_atomic_cmpxchg_i64(val, taddr, llval, tval,
-- 
2.31.1

Re: [PATCH 1/5] target/mips: Replace GET_OFFSET() macro by get_offset() function

[Richard Henderson]

On 8/18/21 6:43 AM, Philippe Mathieu-Daudé wrote:
> The target endianess information is stored in the BigEndian
> bit of the Config0 register in CP0.
> 
> As a first step, replace the GET_OFFSET() macro by an inlined
> get_offset() function, passing CPUMIPSState as argument.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>   target/mips/tcg/ldst_helper.c | 57 +++++++++++++++++++++--------------
>   1 file changed, 35 insertions(+), 22 deletions(-)
> 
> diff --git a/target/mips/tcg/ldst_helper.c b/target/mips/tcg/ldst_helper.c
> index d42812b8a6a..97e7ad7d7a4 100644
> --- a/target/mips/tcg/ldst_helper.c
> +++ b/target/mips/tcg/ldst_helper.c
> @@ -52,31 +52,44 @@ HELPER_LD_ATOMIC(lld, ldq, 0x7, (target_ulong))
>   
>   #endif /* !CONFIG_USER_ONLY */
>   
> +static inline bool cpu_is_bigendian(CPUMIPSState *env)
> +{
> +    return extract32(env->CP0_Config0, CP0C0_BE, 1);
> +}
> +
>   #ifdef TARGET_WORDS_BIGENDIAN
>   #define GET_LMASK(v) ((v) & 3)
> -#define GET_OFFSET(addr, offset) (addr + (offset))
>   #else
>   #define GET_LMASK(v) (((v) & 3) ^ 3)
> -#define GET_OFFSET(addr, offset) (addr - (offset))
>   #endif
>   
> +static inline target_ulong get_offset(CPUMIPSState *env,
> +                                      target_ulong addr, int offset)
> +{
> +    if (cpu_is_bigendian(env)) {
> +        return addr + offset;
> +    } else {
> +        return addr - offset;
> +    }
> +}
> +
>   void helper_swl(CPUMIPSState *env, target_ulong arg1, target_ulong arg2,
>                   int mem_idx)
>   {
>       cpu_stb_mmuidx_ra(env, arg2, (uint8_t)(arg1 >> 24), mem_idx, GETPC());
>   
>       if (GET_LMASK(arg2) <= 2) {
> -        cpu_stb_mmuidx_ra(env, GET_OFFSET(arg2, 1), (uint8_t)(arg1 >> 16),
> +        cpu_stb_mmuidx_ra(env, get_offset(env, arg2, 1), (uint8_t)(arg1 >> 16),
>                             mem_idx, GETPC());
>       }
>   
>       if (GET_LMASK(arg2) <= 1) {
> -        cpu_stb_mmuidx_ra(env, GET_OFFSET(arg2, 2), (uint8_t)(arg1 >> 8),
> +        cpu_stb_mmuidx_ra(env, get_offset(env, arg2, 2), (uint8_t)(arg1 >> 8),
>                             mem_idx, GETPC());

So... yes, this is an improvement, but it's now substituting a constant for a runtime 
variable many times over.  I think you should drop get_offset() entirely and replace it with

     int dir = cpu_is_bigendian(env) ? 1 : -1;

     stb(env, arg2 + 1 * dir, data);

     stb(env, arg2 + 2 * dir, data);

Alternately, bite the bullet and split the function(s) into two, explicitly endian 
versions: helper_swl_be, helper_swl_le, etc.


r~

Re: [PATCH 2/5] target/mips: Replace GET_LMASK() macro by get_lmask(32) function

[Richard Henderson]

On 8/18/21 6:43 AM, Philippe Mathieu-Daudé wrote:
> -    if (GET_LMASK(arg2) <= 2) {
> +    if (get_lmask(env, arg2, 32) <= 2) {

Whatever you decide to do with respect to the previous patch, the result of get_lmask is 
constant across the function and should be computed only once.


r~

Re: [PATCH 4/5] target/mips: Store CP0_Config0 in DisasContext

[Richard Henderson]

On 8/18/21 6:43 AM, Philippe Mathieu-Daudé wrote:
> Most TCG helpers only have access to a DisasContext pointer,
> not CPUMIPSState. Store a copy of CPUMIPSState::CP0_Config0
> in DisasContext so we can access it from TCG helpers.
> 
> Signed-off-by: Philippe Mathieu-Daudé<f4bug@amsat.org>
> ---
>   target/mips/tcg/translate.h | 1 +
>   target/mips/tcg/translate.c | 1 +
>   2 files changed, 2 insertions(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 5/5] target/mips: Replace TARGET_WORDS_BIGENDIAN by cpu_is_bigendian()

[Richard Henderson]

On 8/18/21 6:43 AM, Philippe Mathieu-Daudé wrote:
> Add the inlined cpu_is_bigendian() function in "translate.h".
> 
> Replace the TARGET_WORDS_BIGENDIAN #ifdef'ry by calls to
> cpu_is_bigendian().
> 
> Signed-off-by: Philippe Mathieu-Daudé<f4bug@amsat.org>
> ---
>   target/mips/tcg/translate.h              |  5 ++
>   target/mips/tcg/translate.c              | 70 ++++++++++++------------
>   target/mips/tcg/nanomips_translate.c.inc | 20 +++----
>   3 files changed, 50 insertions(+), 45 deletions(-)
> 

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 2/5] target/mips: Replace GET_LMASK() macro by get_lmask(32) function

[Philippe Mathieu-Daudé]

On 8/18/21 7:09 PM, Richard Henderson wrote:
> On 8/18/21 6:43 AM, Philippe Mathieu-Daudé wrote:
>> -    if (GET_LMASK(arg2) <= 2) {
>> +    if (get_lmask(env, arg2, 32) <= 2) {
> 
> Whatever you decide to do with respect to the previous patch, the result
> of get_lmask is constant across the function and should be computed only
> once.

Oops I missed that, thanks.

Re: [PATCH 1/5] target/mips: Replace GET_OFFSET() macro by get_offset() function

[Philippe Mathieu-Daudé]

On 8/18/21 6:56 PM, Richard Henderson wrote:
> On 8/18/21 6:43 AM, Philippe Mathieu-Daudé wrote:
>> The target endianess information is stored in the BigEndian
>> bit of the Config0 register in CP0.
>>
>> As a first step, replace the GET_OFFSET() macro by an inlined
>> get_offset() function, passing CPUMIPSState as argument.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>   target/mips/tcg/ldst_helper.c | 57 +++++++++++++++++++++--------------
>>   1 file changed, 35 insertions(+), 22 deletions(-)
>>
>> diff --git a/target/mips/tcg/ldst_helper.c
>> b/target/mips/tcg/ldst_helper.c
>> index d42812b8a6a..97e7ad7d7a4 100644
>> --- a/target/mips/tcg/ldst_helper.c
>> +++ b/target/mips/tcg/ldst_helper.c
>> @@ -52,31 +52,44 @@ HELPER_LD_ATOMIC(lld, ldq, 0x7, (target_ulong))
>>     #endif /* !CONFIG_USER_ONLY */
>>   +static inline bool cpu_is_bigendian(CPUMIPSState *env)
>> +{
>> +    return extract32(env->CP0_Config0, CP0C0_BE, 1);
>> +}
>> +
>>   #ifdef TARGET_WORDS_BIGENDIAN
>>   #define GET_LMASK(v) ((v) & 3)
>> -#define GET_OFFSET(addr, offset) (addr + (offset))
>>   #else
>>   #define GET_LMASK(v) (((v) & 3) ^ 3)
>> -#define GET_OFFSET(addr, offset) (addr - (offset))
>>   #endif
>>   +static inline target_ulong get_offset(CPUMIPSState *env,
>> +                                      target_ulong addr, int offset)
>> +{
>> +    if (cpu_is_bigendian(env)) {
>> +        return addr + offset;
>> +    } else {
>> +        return addr - offset;
>> +    }
>> +}
>> +
>>   void helper_swl(CPUMIPSState *env, target_ulong arg1, target_ulong
>> arg2,
>>                   int mem_idx)
>>   {
>>       cpu_stb_mmuidx_ra(env, arg2, (uint8_t)(arg1 >> 24), mem_idx,
>> GETPC());
>>         if (GET_LMASK(arg2) <= 2) {
>> -        cpu_stb_mmuidx_ra(env, GET_OFFSET(arg2, 1), (uint8_t)(arg1 >>
>> 16),
>> +        cpu_stb_mmuidx_ra(env, get_offset(env, arg2, 1),
>> (uint8_t)(arg1 >> 16),
>>                             mem_idx, GETPC());
>>       }
>>         if (GET_LMASK(arg2) <= 1) {
>> -        cpu_stb_mmuidx_ra(env, GET_OFFSET(arg2, 2), (uint8_t)(arg1 >>
>> 8),
>> +        cpu_stb_mmuidx_ra(env, get_offset(env, arg2, 2),
>> (uint8_t)(arg1 >> 8),
>>                             mem_idx, GETPC());
> 
> So... yes, this is an improvement, but it's now substituting a constant
> for a runtime variable many times over.

Oops indeed.

>  I think you should drop
> get_offset() entirely and replace it with
> 
>     int dir = cpu_is_bigendian(env) ? 1 : -1;
> 
>     stb(env, arg2 + 1 * dir, data);
> 
>     stb(env, arg2 + 2 * dir, data);
> 
> Alternately, bite the bullet and split the function(s) into two,
> explicitly endian versions: helper_swl_be, helper_swl_le, etc.

I'll go for the easier path ;)

Re: [PATCH 1/5] target/mips: Replace GET_OFFSET() macro by get_offset() function

[Richard Henderson]

On 8/18/21 11:31 AM, Philippe Mathieu-Daudé wrote:
>>    I think you should drop
>> get_offset() entirely and replace it with
>>
>>      int dir = cpu_is_bigendian(env) ? 1 : -1;
>>
>>      stb(env, arg2 + 1 * dir, data);
>>
>>      stb(env, arg2 + 2 * dir, data);
>>
>> Alternately, bite the bullet and split the function(s) into two,
>> explicitly endian versions: helper_swl_be, helper_swl_le, etc.
> 
> I'll go for the easier path ;)

It's not really more difficult.

static inline void do_swl(env, uint32_t val, target_ulong addr, int midx,
                           int dir, unsigned lmask, uintptr_t ra)
{
     cpu_stb_mmuidx_ra(env, addr, val >> 24, midx, ra);

     if (lmask <= 2) {
         cpu_stb_mmuidx_ra(env, addr + 1 * dir, val >> 16, midx, ra);
     }
     if (lmask <= 1) {
         cpu_stb_mmuidx_ra(env, addr + 1 * dir, val >> 8, midx, ra);
     }
     if (lmask == 0) {
         cpu_stb_mmuidx_ra(env, addr + 1 * dir, val, midx, ra);
     }
}

void helper_swl_be(env, val, addr, midx)
{
     do_swl(env, val, addr, midx, 1, addr & 3, GETPC());
}

void helper_swl_le(env, val, addr, midx)
{
     do_swl(env, val, addr, midx, -1, ~addr & 3, GETPC());
}

Although I do wonder if this is strictly correct with respect to atomicity.  In my 
tcg/mips unaligned patch set, I assumed that lwl+lwr of an aligned address produces two 
atomic 32-bit loads, which result in a complete atomic load at the end.

Should we be doing something like

void helper_swl_be(env, val, addr, midx)
{
     uintptr_t ra = GETPC();

     switch (addr & 3) {
     case 0:
         cpu_stl_be_mmuidx_ra(env, val, addr, midx, ra);
         break;
     case 1:
         cpu_stb_mmuidx_ra(env, val >> 24, addr, midx, ra);
         cpu_stw_be_mmuidx_ra(env, val >> 16, addr + 1, midx, ra);
         break;
     case 2:
         cpu_stw_be_mmuidx_ra(env, val >> 16, addr, midx, ra);
         break;
     case 3:
         cpu_stb_mmuidx_ra(env, val >> 24, addr, midx, ra);
         break;
     }
}

void helper_swl_le(env, val, addr, midx)
{
     uintptr_t ra = GETPC();

     /*
      * We want to use stw and stl for atomicity, but want any
      * fault to report ADDR, not the aligned address.
      */
     probe_write(env, addr, 0, midx, ra);

     switch (addr & 3) {
     case 3:
         cpu_stl_le_mmuidx_ra(env, val, addr - 3, midx, ra);
         break;
     case 1:
         cpu_stw_le_mmuidx_ra(env, val >> 16, addr - 1, midx, ra);
         break;
     case 2:
         cpu_stw_le_mmuidx_ra(env, val >> 8, addr - 2, midx, ra);
         /* fall through */
     case 0:
         cpu_stb_mmuidx_ra(env, val >> 24, addr, midx, ra);
         break;
     }
}

etc.


r~