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

[Heiko Stuebner <heiko@sntech.de>]

Am Mittwoch, 7. Dezember 2022, 21:07:39 CET schrieb Conor Dooley:
> 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?

will do

> Otherwise:
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> I do like the comments a lot :) Saves going off to read specs...

That was the intention with the big comments :-) .

And self-preservation ... Once my mind does a cache-flush of these things,
I don't want to have to dig through things to re-understand this again.


Heiko



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