[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v7 09/14] target/riscv: rvk: add support for sha512 related i

From: Richard Henderson
Subject: Re: [PATCH v7 09/14] target/riscv: rvk: add support for sha512 related instructions for RV32 in zknh extension
Date: Mon, 28 Feb 2022 09:38:52 -1000
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.5.0

On 2/28/22 04:48, Weiwei Li wrote:
+#define GEN_SHA512H_RV32(NAME, OP, NUM1, NUM2, NUM3) \
+static void gen_##NAME(TCGv dest, TCGv src1, TCGv src2) \
+{ \
+    TCGv_i64 t0 = tcg_temp_new_i64(); \
+    TCGv_i64 t1 = tcg_temp_new_i64(); \
+    TCGv_i64 t2 = tcg_temp_new_i64(); \
+    \
+    tcg_gen_concat_tl_i64(t0, src1, src2); \
+    tcg_gen_##OP##_i64(t1, t0, NUM1); \
+    tcg_gen_concat_tl_i64(t2, src1, tcg_const_tl(0)); \

The bug here is tcg_const_tl instead of tcg_constant_tl, which leaks a 

It's not the best option for zero-extension, though, as we don't optimize a deposit of zero like this (we probably should, but, hey).

Better would be

    tcg_gen_extu_tl_i64(t2, src1);
    tcg_gen_ext32u_i64(t2, t2);

Note that the first operation will *not* extend if TARGET_RISCV64, since it doesn't actually change type. The second operation will be optimized away if TARGET_RISCV32, since the zero-extend has already happened.

BTW, it would be better to not use a large macro for this, and in the previous patch. Passing in parameters to a helper function would be easier to read and debug.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]