Re: [PATCH v3 08/17] riscv: Use generated instruction headers for misaligned loads/stores

Charlie Jenkins <[email protected]> writes:
> -static int handle_scalar_misaligned_load(struct pt_regs *regs)
> +static noinline int handle_scalar_misaligned_load(struct pt_regs *regs)

Why noinline? I heard that compilers are usually better than human at
deciding which functions to inline.

> +	if (rd) {
> +		if (!fp)
> +			*(unsigned long *)((unsigned long *)regs + rd) = (val.data_ulong << shift) >> shift;
> +		else if (len == 8)
> +			set_f64_rd(rd, regs, val.data_u64);
> +		else
> +			set_f32_rd(rd, regs, val.data_ulong);

Why if (rd) condition? What if the instruction is loading to f0?

> +		rd = (8 + riscv_insn_c_sh_extract_xs2(insn));

Having to remember when to +8 is error-prone. Does it make more sense to
do the +8 inside riscv_insn_c_sh_extract_xs2() and the likes?

> +	if (rd) {
> +		if (!fp)
> +			val.data_ulong = *(unsigned long *)((unsigned long *)regs + rd);
> +		else if (len == 8)
> +			val.data_u64 = get_f64_rs(rd, regs);
> +		else
> +			val.data_ulong = get_f32_rs(rd, regs);
> +
> +		if (user_mode(regs)) {
> +			if (copy_to_user((u8 __user *)addr, &val, len))
> +				return -1;
> +		} else {
> +			memcpy((u8 *)addr, &val, len);
> +		}
>  	}

Again, why the if (rd) condition? What if it is f0?

Nam

-- 
kvm-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kvm-riscv