Re: [PATCH v6 55/61] target/riscv: integer extract instruction

[LIU Zhiwei]

On 2020/3/28 11:36, Richard Henderson wrote:

On 3/17/20 8:06 AM, LIU Zhiwei wrote:

+/* Integer Extract Instruction */
+static void extract_element(TCGv dest, TCGv_ptr base,
+int ofs, int sew)
+{
+switch (sew) {
+case MO_8:
+tcg_gen_ld8u_tl(dest, base, ofs);
+break;
+case MO_16:
+tcg_gen_ld16u_tl(dest, base, ofs);
+break;
+default:
+tcg_gen_ld32u_tl(dest, base, ofs);
+break;
+#if TARGET_LONG_BITS == 64
+case MO_64:
+tcg_gen_ld_i64(dest, base, ofs);
+break;
+#endif
+}
+}

I just remembered that this doesn't handle HOST_WORDS_BIGENDIAN properly -- the
MO_64 case for TARGET_LONG_BITS == 32.

Because we computed the offset for MO_64, not MO_32, we need

 case MO_64:
 if (TARGET_LONG_BITS == 64) {
 tcg_gen_ld_i64(dest, base, ofs);
 break;
 }
#ifdef HOST_WORDS_BIGENDIAN
 ofs += 4;
#endif
 /* fall through */
 case MO_32:
 tcg_gen_ld32u_tl(dest, base, ofs);
 break;
 default:
 g_assert_not_reached();

Yes, it should be.

As extract_element and gather_element are very similar . I
will merge them to  load_element in v7.

Zhiwei


r~

Re: [PATCH v6 55/61] target/riscv: integer extract instruction

[Richard Henderson]

On 3/17/20 8:06 AM, LIU Zhiwei wrote:
> +/* Integer Extract Instruction */
> +static void extract_element(TCGv dest, TCGv_ptr base,
> +int ofs, int sew)
> +{
> +switch (sew) {
> +case MO_8:
> +tcg_gen_ld8u_tl(dest, base, ofs);
> +break;
> +case MO_16:
> +tcg_gen_ld16u_tl(dest, base, ofs);
> +break;
> +default:
> +tcg_gen_ld32u_tl(dest, base, ofs);
> +break;
> +#if TARGET_LONG_BITS == 64
> +case MO_64:
> +tcg_gen_ld_i64(dest, base, ofs);
> +break;
> +#endif
> +}
> +}

I just remembered that this doesn't handle HOST_WORDS_BIGENDIAN properly -- the
MO_64 case for TARGET_LONG_BITS == 32.

Because we computed the offset for MO_64, not MO_32, we need

case MO_64:
if (TARGET_LONG_BITS == 64) {
tcg_gen_ld_i64(dest, base, ofs);
break;
}
#ifdef HOST_WORDS_BIGENDIAN
ofs += 4;
#endif
/* fall through */
case MO_32:
tcg_gen_ld32u_tl(dest, base, ofs);
break;
default:
g_assert_not_reached();


r~