Re: [PATCH v2 37/55] target/mips: Use 8-byte memory ops for msa load/store

[Richard Henderson]

On 8/17/21 11:21 PM, Philippe Mathieu-Daudé wrote:

+#ifdef TARGET_WORDS_BIGENDIAN
+static inline uint64_t bswap16x4(uint64_t x)
+{
+    uint64_t m = 0x00ff00ff00ff00ffull;
+    return ((x & m) << 8) | ((x >> 8) & m);
+}
+
+static inline uint64_t bswap32x2(uint64_t x)
+{
+    return ror64(bswap64(x), 32);
+}
+#endif

I'm trying to remove TARGET_WORDS_BIGENDIAN uses,
so this would become:

static inline bool is_cpu_bigendian(CPUMIPSState *)
{
     return extract32(env->CP0_Config0, CP0C0_BE, 1);
}

static inline uint64_t bswap16x4(CPUMIPSState *env, uint64_t x)
{
     if (is_cpu_bigendian(env)) {
         uint64_t m = 0x00ff00ff00ff00ffull;
         return ((x & m) << 8) | ((x >> 8) & m);
     } else {
         return x;
     }
}

static inline uint64_t bswap32x2(CPUMIPSState *env, uint64_t x)
{
     if (is_cpu_bigendian(env)) {
         return ror64(bswap64(x), 32);
     } else {
         return x;
     }
}

I would not put the bigendian test in here...

And we can remove the other TARGET_WORDS_BIGENDIAN uses:

  void helper_msa_ld_w(CPUMIPSState *env, uint32_t wd,
@@ -8293,18 +8271,20 @@ void helper_msa_ld_w(CPUMIPSState *env, uint32_t wd,
  {
      wr_t *pwd = &(env->active_fpu.fpr[wd].wr);
      uintptr_t ra = GETPC();
+    uint64_t d0, d1;

-#if !defined(HOST_WORDS_BIGENDIAN)

-    pwd->w[0] = cpu_ldl_data_ra(env, addr + (0 << DF_WORD), ra);
-    pwd->w[1] = cpu_ldl_data_ra(env, addr + (1 << DF_WORD), ra);
-    pwd->w[2] = cpu_ldl_data_ra(env, addr + (2 << DF_WORD), ra);
-    pwd->w[3] = cpu_ldl_data_ra(env, addr + (3 << DF_WORD), ra);
-#else
-    pwd->w[0] = cpu_ldl_data_ra(env, addr + (1 << DF_WORD), ra);
-    pwd->w[1] = cpu_ldl_data_ra(env, addr + (0 << DF_WORD), ra);
-    pwd->w[2] = cpu_ldl_data_ra(env, addr + (3 << DF_WORD), ra);
-    pwd->w[3] = cpu_ldl_data_ra(env, addr + (2 << DF_WORD), ra);
+    /*
+     * Load 8 bytes at a time.  Use little-endian load, then for
+     * big-endian target, we must then bswap the two words.
+     */
+    d0 = cpu_ldq_le_data_ra(env, addr + 0, ra);
+    d1 = cpu_ldq_le_data_ra(env, addr + 8, ra);
+#ifdef TARGET_WORDS_BIGENDIAN
+    d0 = bswap32x2(d0);
+    d1 = bswap32x2(d1);
  #endif

... I would leave it here.


r~