Re: [PATCH v4 11/12] RISC-V: add helpers for handling immediates in U-type and I-type pairs

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1.1: Type: text/plain, Size: 3418 bytes --]

On Wed, Dec 07, 2022 at 07:08:20PM +0100, Heiko Stuebner wrote:
> From: Heiko Stuebner <heiko.stuebner@vrull.eu>
> 
> Used together U-type and I-type instructions can for example be used to
> generate bigger jumps (i.e. in auipc+jalr pairs) by splitting the value
> into an upper immediate (i.e. auipc) and a 12bit immediate (i.e. jalr).
> 
> Due to both immediates being considered signed this creates some corner
> cases, so add some helper to prevent this from getting duplicated in
> different places.
> 
> Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
> ---
>  arch/riscv/include/asm/insn.h | 47 +++++++++++++++++++++++++++++++++++
>  1 file changed, 47 insertions(+)
> 
> diff --git a/arch/riscv/include/asm/insn.h b/arch/riscv/include/asm/insn.h
> index 2a23890b4577..bb1e6120a560 100644
> --- a/arch/riscv/include/asm/insn.h
> +++ b/arch/riscv/include/asm/insn.h
> @@ -290,3 +290,50 @@ static __always_inline bool riscv_insn_is_branch(u32 code)
>  	(RVC_X(x_, RVC_B_IMM_5_OPOFF, RVC_B_IMM_5_MASK) << RVC_B_IMM_5_OFF) | \
>  	(RVC_X(x_, RVC_B_IMM_7_6_OPOFF, RVC_B_IMM_7_6_MASK) << RVC_B_IMM_7_6_OFF) | \
>  	(RVC_IMM_SIGN(x_) << RVC_B_IMM_SIGN_OFF); })
> +
> +/*
> + * Put together one immediate from a U-type and I-type instruction pair.
> + *
> + * The U-type contains an upper immediate, meaning bits[31:12] with [11:0]
> + * being zero, while the I-type contains a 12bit immediate.
> + * Combined these can encode larger 32bit values and are used for example
> + * in auipc + jalr pairs to allow larger jumps.
> + *
> + * @utype_insn: instruction containing the upper immediate
> + * @itype_insn: instruction
> + * Return: combined immediate
> + */
> +static inline s32 riscv_insn_extract_utype_itype_imm(u32 utype_insn, u32 itype_insn)
> +{
> +	s32 imm;
> +
> +	imm = RV_EXTRACT_UTYPE_IMM(utype_insn);
> +	imm += RV_EXTRACT_ITYPE_IMM(itype_insn);
> +
> +	return imm;
> +}
> +
> +/*
> + * Update a set of two instructions (U-type + I-type) with an immediate value.
> + *
> + * Used for example in auipc+jalrs pairs the U-type instructions contains
> + * a 20bit upper immediate representing bits[31:12], while the I-type
> + * instruction contains a 12bit immediate representing bits[11:0].
> + *
> + * This also takes into account that both separate immediates are
> + * considered as signed values, so if the I-type immediate becomes
> + * negative (BIT(11) set) the U-type part gets adjusted.
> + *
> + * @insn: pointer to a set of two instructions
> + * @imm: the immediate to insert into the two instructions
> + */
> +static inline void riscv_insn_insert_utype_itype_imm(u32 *insn, s32 imm)
> +{
> +	/* drop possible old IMM values */
> +	insn[0] &= ~(RV_U_IMM_31_12_MASK);
> +	insn[1] &= ~(RV_I_IMM_11_0_MASK << RV_I_IMM_11_0_OPOFF);
> +
> +	/* add the adapted IMMs */
> +	insn[0] |= ((imm & RV_U_IMM_31_12_MASK) + ((imm & BIT(11)) << 1));
> +	insn[1] |= ((imm & RV_I_IMM_11_0_MASK) << RV_I_IMM_11_0_OPOFF);

If you send a v5, could you drop the extra ()s around some of these?
Otherwise:
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
I do like the comments a lot :) Saves going off to read specs...

> +}
> -- 
> 2.35.1
> 
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv