Re: [PATCH v4 10/21] target/loongarch: Add floating point arithmetic instruction translation

[Richard Henderson]

On 9/2/21 2:40 PM, Song Gao wrote:
+const FloatRoundMode ieee_rm[4] = {

Make static.

+int ieee_ex_to_loongarch(int xcpt)

This function is only used in this file.  Better to make it static and remove the 
declaration from translate.h, which allows you to remove that include.

+static inline void update_fcsr0(CPULoongArchState *env, uintptr_t pc)

Remove inline.

+{
+    int flags = ieee_ex_to_loongarch(get_float_exception_flags(
+                                  &env->fp_status));
+
+    SET_FP_CAUSE(env->fcsr0, flags);
+    if (flags) {

Hmm.  Between the two functions you're testing for zero twice.  Perhaps better 
to rearrange as

{
    int flags = get_float_exception_flags(&env->fp_status);

    if (!flags) {
        SET_FP_CAUSE(env->fcsr0, 0);
        return;
    }

    set_float_exception_flags(0, &env->fp_status);
    flags = ieee_ex_to_loongarch(flags);
    SET_FP_CAUSE(env->fcsr0, flags);

    if (GET_FP_ENABLE(env->fcsr0) & flags) {
        do_raise_exception(env, EXCP_FPE, pc);
    }
    UPDATE_FP_FLAGS(env->fcsr0, flags);
}

and remove the explicit zero test in ieee_ex_to_loongarch.

+uint64_t helper_flogb_s(CPULoongArchState *env, uint64_t fj)
+{
+    uint64_t fd;
+    uint32_t fp;
+    float_status *status = &env->fp_status;
+    FloatRoundMode old_mode = get_float_rounding_mode(status);
+
+    set_float_exception_flags(0, status);

Unnecessary, since the steady-state is already zero.

+    set_float_exception_flags(get_float_exception_flags(status) &
+                              (~float_flag_inexact), status);
+    update_fcsr0(env, GETPC());

Hmm.  Worth adding a parameter to update_fcsr0, a mask to suppress?  Or a common 
subroutine like

    update_fcsr0_mask(env, GETPC(), float_flag_inexact);

static void update_fcsr0(env, ra)
{
    update_fcsr0_mask(env, ra, 0);
}

+static bool trans_fcopysign_s(DisasContext *ctx, arg_fmt_fdfjfk *a)
+{
+    tcg_gen_deposit_i64(cpu_fpr[a->fd], cpu_fpr[a->fk], cpu_fpr[a->fj], 0, 31);
+    return true;
+}
+
+static bool trans_fcopysign_d(DisasContext *ctx, arg_fmt_fdfjfk *a)
+{
+    tcg_gen_deposit_i64(cpu_fpr[a->fd], cpu_fpr[a->fk], cpu_fpr[a->fj], 0, 63);
+    return true;
+}

For what it's worth, you could treat the abs and neg instructions similarly, 
inline.


r~