diff mbox series

[v2,02/13] RISC-V: detach funct-values from their offset

Message ID 20221128102632.435174-3-heiko@sntech.de (mailing list archive)
State Superseded
Delegated to: Palmer Dabbelt
Headers show
Series Zbb string optimizations and call support in alternatives | expand

Checks

Context Check Description
conchuod/tree_selection fail Guessing tree name failed

Commit Message

Heiko Stuebner Nov. 28, 2022, 10:26 a.m. UTC 
From: Heiko Stuebner <heiko.stuebner@vrull.eu>

Only carry the actual values for the funct3, funct4, etc instruction
fields without directly including the shift to the target position.

Instead use macros to move the values to their target positions
when needed.

This at the same time also reduces the use of magic numbers,
one would need a spec manual to understand.

Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
---
 arch/riscv/include/asm/parse_asm.h | 100 +++++++++++++++++------------
 1 file changed, 59 insertions(+), 41 deletions(-)

Comments

Conor Dooley Nov. 29, 2022, 10:47 p.m. UTC | #1 
On Mon, Nov 28, 2022 at 11:26:21AM +0100, Heiko Stuebner wrote:
> From: Heiko Stuebner <heiko.stuebner@vrull.eu>
> 
> Only carry the actual values for the funct3, funct4, etc instruction
> fields without directly including the shift to the target position.

"Only carry the actual values..." didn't really make sense to me until I
actually looked at the diff. I'm not sure what "better" wording to
suggest though... "Rather than defining funct3 (...) etc instruction
fields, pre-shifted to their target positions, define the values
themselves."

Is that better? I don't know, but at least I didn't say "I hate this"
without trying to help :)

> Instead use macros to move the values to their target positions
> when needed.
> 
> This at the same time also reduces the use of magic numbers,
> one would need a spec manual to understand.

I'm always down to avoid having to read a spec :)

> Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
> ---
>  arch/riscv/include/asm/parse_asm.h | 100 +++++++++++++++++------------
>  1 file changed, 59 insertions(+), 41 deletions(-)
> 
> diff --git a/arch/riscv/include/asm/parse_asm.h b/arch/riscv/include/asm/parse_asm.h
> index c52b8ae02cfe..e3f87da108f4 100644
> --- a/arch/riscv/include/asm/parse_asm.h
> +++ b/arch/riscv/include/asm/parse_asm.h
> @@ -5,6 +5,15 @@
>  
>  #include <linux/bits.h>
>  
> +#define RV_INSN_FUNCT3_MASK	GENMASK(14, 12)
> +#define RV_INSN_FUNCT3_OPOFF	12
> +#define RV_INSN_OPCODE_MASK	GENMASK(6, 0)
> +#define RV_INSN_OPCODE_OPOFF	0
> +#define RV_INSN_FUNCT12_OPOFF	20
> +
> +#define RV_ENCODE_FUNCT3(f_)	(RVG_FUNCT3_##f_ << RV_INSN_FUNCT3_OPOFF)
> +#define RV_ENCODE_FUNCT12(f_)	(RVG_FUNCT12_##f_ << RV_INSN_FUNCT12_OPOFF)
> +
>  /* The bit field of immediate value in I-type instruction */
>  #define RV_I_IMM_SIGN_OPOFF	31
>  #define RV_I_IMM_11_0_OPOFF	20
> @@ -84,6 +93,15 @@
>  #define RVC_B_IMM_2_1_MASK	GENMASK(1, 0)
>  #define RVC_B_IMM_5_MASK	GENMASK(0, 0)
>  
> +#define RVC_INSN_FUNCT4_MASK	GENMASK(15, 12)
> +#define RVC_INSN_FUNCT4_OPOFF	12
> +#define RVC_INSN_FUNCT3_MASK	GENMASK(15, 13)
> +#define RVC_INSN_FUNCT3_OPOFF	13
> +#define RVC_INSN_J_RS2_MASK	GENMASK(6, 2)

Since I won't feel comfortable reviewing this without checking the
specs, I did and god is Table 1.1 in the v1.7 compressed spec
horrifically formatted. The numbers aren't even consistently spaced.
My poor OCD! I thought they made these things with adoc or LaTeX...

> +#define RVC_INSN_OPCODE_MASK	GENMASK(1, 0)
> +#define RVC_ENCODE_FUNCT3(f_)	(RVC_FUNCT3_##f_ << RVC_INSN_FUNCT3_OPOFF)
> +#define RVC_ENCODE_FUNCT4(f_)	(RVC_FUNCT4_##f_ << RVC_INSN_FUNCT4_OPOFF)
> +
>  /* The register offset in RVC op=C0 instruction */
>  #define RVC_C0_RS1_OPOFF	7
>  #define RVC_C0_RS2_OPOFF	2
> @@ -113,52 +131,52 @@
>  /* parts of funct3 code for I, M, A extension*/
>  #define RVG_FUNCT3_JALR		0x0
>  #define RVG_FUNCT3_BEQ		0x0
> -#define RVG_FUNCT3_BNE		0x1000
> -#define RVG_FUNCT3_BLT		0x4000
> -#define RVG_FUNCT3_BGE		0x5000
> -#define RVG_FUNCT3_BLTU		0x6000
> -#define RVG_FUNCT3_BGEU		0x7000
> +#define RVG_FUNCT3_BNE		0x1
> +#define RVG_FUNCT3_BLT		0x4
> +#define RVG_FUNCT3_BGE		0x5
> +#define RVG_FUNCT3_BLTU		0x6
> +#define RVG_FUNCT3_BGEU		0x7
>  
>  /* parts of funct3 code for C extension*/
> -#define RVC_FUNCT3_C_BEQZ	0xc000
> -#define RVC_FUNCT3_C_BNEZ	0xe000
> -#define RVC_FUNCT3_C_J		0xa000
> -#define RVC_FUNCT3_C_JAL	0x2000
> -#define RVC_FUNCT4_C_JR		0x8000
> -#define RVC_FUNCT4_C_JALR	0xf000
> +#define RVC_FUNCT3_C_BEQZ	0x6
> +#define RVC_FUNCT3_C_BNEZ	0x7
> +#define RVC_FUNCT3_C_J		0x5
> +#define RVC_FUNCT3_C_JAL	0x1
> +#define RVC_FUNCT4_C_JR		0x8
> +#define RVC_FUNCT4_C_JALR	0x9
>  
> -#define RVG_FUNCT12_SRET	0x10200000
> +#define RVG_FUNCT12_SRET	0x102

>  
> -#define RVG_MATCH_JALR		(RVG_FUNCT3_JALR | RVG_OPCODE_JALR)
> +#define RVG_MATCH_JALR		(RV_ENCODE_FUNCT3(JALR) | RVG_OPCODE_JALR)
>  #define RVG_MATCH_JAL		(RVG_OPCODE_JAL)
> -#define RVG_MATCH_BEQ		(RVG_FUNCT3_BEQ | RVG_OPCODE_BRANCH)
> -#define RVG_MATCH_BNE		(RVG_FUNCT3_BNE | RVG_OPCODE_BRANCH)
> -#define RVG_MATCH_BLT		(RVG_FUNCT3_BLT | RVG_OPCODE_BRANCH)
> -#define RVG_MATCH_BGE		(RVG_FUNCT3_BGE | RVG_OPCODE_BRANCH)
> -#define RVG_MATCH_BLTU		(RVG_FUNCT3_BLTU | RVG_OPCODE_BRANCH)
> -#define RVG_MATCH_BGEU		(RVG_FUNCT3_BGEU | RVG_OPCODE_BRANCH)
> -#define RVG_MATCH_SRET		(RVG_FUNCT12_SRET | RVG_OPCODE_SYSTEM)
> -#define RVC_MATCH_C_BEQZ	(RVC_FUNCT3_C_BEQZ | RVC_OPCODE_C1)
> -#define RVC_MATCH_C_BNEZ	(RVC_FUNCT3_C_BNEZ | RVC_OPCODE_C1)
> -#define RVC_MATCH_C_J		(RVC_FUNCT3_C_J | RVC_OPCODE_C1)
> -#define RVC_MATCH_C_JAL		(RVC_FUNCT3_C_JAL | RVC_OPCODE_C1)
> -#define RVC_MATCH_C_JR		(RVC_FUNCT4_C_JR | RVC_OPCODE_C2)
> -#define RVC_MATCH_C_JALR	(RVC_FUNCT4_C_JALR | RVC_OPCODE_C2)
> -
> -#define RVG_MASK_JALR		0x707f
> -#define RVG_MASK_JAL		0x7f
> -#define RVC_MASK_C_JALR		0xf07f
> -#define RVC_MASK_C_JR		0xf07f
> -#define RVC_MASK_C_JAL		0xe003
> -#define RVC_MASK_C_J		0xe003
> -#define RVG_MASK_BEQ		0x707f
> -#define RVG_MASK_BNE		0x707f
> -#define RVG_MASK_BLT		0x707f
> -#define RVG_MASK_BGE		0x707f
> -#define RVG_MASK_BLTU		0x707f
> -#define RVG_MASK_BGEU		0x707f
> -#define RVC_MASK_C_BEQZ		0xe003
> -#define RVC_MASK_C_BNEZ		0xe003
> +#define RVG_MATCH_BEQ		(RV_ENCODE_FUNCT3(BEQ) | RVG_OPCODE_BRANCH)
> +#define RVG_MATCH_BNE		(RV_ENCODE_FUNCT3(BNE) | RVG_OPCODE_BRANCH)
> +#define RVG_MATCH_BLT		(RV_ENCODE_FUNCT3(BLT) | RVG_OPCODE_BRANCH)
> +#define RVG_MATCH_BGE		(RV_ENCODE_FUNCT3(BGE) | RVG_OPCODE_BRANCH)
> +#define RVG_MATCH_BLTU		(RV_ENCODE_FUNCT3(BLTU) | RVG_OPCODE_BRANCH)
> +#define RVG_MATCH_BGEU		(RV_ENCODE_FUNCT3(BGEU) | RVG_OPCODE_BRANCH)
> +#define RVG_MATCH_SRET		(RV_ENCODE_FUNCT12(SRET) | RVG_OPCODE_SYSTEM)
> +#define RVC_MATCH_C_BEQZ	(RVC_ENCODE_FUNCT3(C_BEQZ) | RVC_OPCODE_C1)
> +#define RVC_MATCH_C_BNEZ	(RVC_ENCODE_FUNCT3(C_BNEZ) | RVC_OPCODE_C1)
> +#define RVC_MATCH_C_J		(RVC_ENCODE_FUNCT3(C_J) | RVC_OPCODE_C1)
> +#define RVC_MATCH_C_JAL		(RVC_ENCODE_FUNCT3(C_JAL) | RVC_OPCODE_C1)
> +#define RVC_MATCH_C_JR		(RVC_ENCODE_FUNCT4(C_JR) | RVC_OPCODE_C2)
> +#define RVC_MATCH_C_JALR	(RVC_ENCODE_FUNCT4(C_JALR) | RVC_OPCODE_C2)
> +
> +#define RVG_MASK_JALR		(RV_INSN_FUNCT3_MASK | RV_INSN_OPCODE_MASK)
> +#define RVG_MASK_JAL		(RV_INSN_OPCODE_MASK)
> +#define RVC_MASK_C_JALR		(RVC_INSN_FUNCT4_MASK | RVC_INSN_J_RS2_MASK | RVC_INSN_OPCODE_MASK)
> +#define RVC_MASK_C_JR		(RVC_INSN_FUNCT4_MASK | RVC_INSN_J_RS2_MASK | RVC_INSN_OPCODE_MASK)
> +#define RVC_MASK_C_JAL		(RVC_INSN_FUNCT3_MASK | RVC_INSN_OPCODE_MASK)
> +#define RVC_MASK_C_J		(RVC_INSN_FUNCT3_MASK | RVC_INSN_OPCODE_MASK)
> +#define RVG_MASK_BEQ		(RV_INSN_FUNCT3_MASK | RV_INSN_OPCODE_MASK)
> +#define RVG_MASK_BNE		(RV_INSN_FUNCT3_MASK | RV_INSN_OPCODE_MASK)
> +#define RVG_MASK_BLT		(RV_INSN_FUNCT3_MASK | RV_INSN_OPCODE_MASK)
> +#define RVG_MASK_BGE		(RV_INSN_FUNCT3_MASK | RV_INSN_OPCODE_MASK)
> +#define RVG_MASK_BLTU		(RV_INSN_FUNCT3_MASK | RV_INSN_OPCODE_MASK)
> +#define RVG_MASK_BGEU		(RV_INSN_FUNCT3_MASK | RV_INSN_OPCODE_MASK)
> +#define RVC_MASK_C_BEQZ		(RVC_INSN_FUNCT3_MASK | RVC_INSN_OPCODE_MASK)
> +#define RVC_MASK_C_BNEZ		(RVC_INSN_FUNCT3_MASK | RVC_INSN_OPCODE_MASK)
>  #define RVG_MASK_SRET		0xffffffff
>  
>  #define __INSN_LENGTH_MASK	_UL(0x3)

My eyes got tired near the end of this, but I had a good through and
spot checked against the values in the spec docs and all came up good
for me. I definitely like the cases where you manage to avoid using
defines that'd require an RE effort to figure out what the actual fields
are.
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
Conor Dooley Nov. 29, 2022, 11:01 p.m. UTC | #2 
Woops, I built this commit in isolation and:
/stuff/linux/arch/riscv/kernel/kgdb.c:32:32: error: use of undeclared identifier 'MASK_JALR'
DECLARE_INSN(jalr, MATCH_JALR, MASK_JALR)
                               ^
/stuff/linux/arch/riscv/kernel/kgdb.c:32:20: error: use of undeclared identifier 'MATCH_JALR'
DECLARE_INSN(jalr, MATCH_JALR, MASK_JALR)
                   ^
/stuff/linux/arch/riscv/kernel/kgdb.c:33:30: error: use of undeclared identifier 'MASK_JAL'
DECLARE_INSN(jal, MATCH_JAL, MASK_JAL)
                             ^
/stuff/linux/arch/riscv/kernel/kgdb.c:33:19: error: use of undeclared identifier 'MATCH_JAL'
DECLARE_INSN(jal, MATCH_JAL, MASK_JAL)
                  ^
/stuff/linux/arch/riscv/kernel/kgdb.c:34:32: error: use of undeclared identifier 'MASK_C_JR'
DECLARE_INSN(c_jr, MATCH_C_JR, MASK_C_JR)
                               ^
/stuff/linux/arch/riscv/kernel/kgdb.c:34:20: error: use of undeclared identifier 'MATCH_C_JR'
DECLARE_INSN(c_jr, MATCH_C_JR, MASK_C_JR)
                   ^
/stuff/linux/arch/riscv/kernel/kgdb.c:35:36: error: use of undeclared identifier 'MASK_C_JALR'
DECLARE_INSN(c_jalr, MATCH_C_JALR, MASK_C_JALR)
                                   ^
/stuff/linux/arch/riscv/kernel/kgdb.c:35:22: error: use of undeclared identifier 'MATCH_C_JALR'
DECLARE_INSN(c_jalr, MATCH_C_JALR, MASK_C_JALR)
                     ^
/stuff/linux/arch/riscv/kernel/kgdb.c:36:30: error: use of undeclared identifier 'MASK_C_J'
DECLARE_INSN(c_j, MATCH_C_J, MASK_C_J)
                             ^
/stuff/linux/arch/riscv/kernel/kgdb.c:36:19: error: use of undeclared identifier 'MATCH_C_J'
DECLARE_INSN(c_j, MATCH_C_J, MASK_C_J)
                  ^
/stuff/linux/arch/riscv/kernel/kgdb.c:37:30: error: use of undeclared identifier 'MASK_BEQ'
DECLARE_INSN(beq, MATCH_BEQ, MASK_BEQ)
                             ^
/stuff/linux/arch/riscv/kernel/kgdb.c:37:19: error: use of undeclared identifier 'MATCH_BEQ'
DECLARE_INSN(beq, MATCH_BEQ, MASK_BEQ)
                  ^
/stuff/linux/arch/riscv/kernel/kgdb.c:38:30: error: use of undeclared identifier 'MASK_BNE'
DECLARE_INSN(bne, MATCH_BNE, MASK_BNE)
                             ^
/stuff/linux/arch/riscv/kernel/kgdb.c:38:19: error: use of undeclared identifier 'MATCH_BNE'
DECLARE_INSN(bne, MATCH_BNE, MASK_BNE)
                  ^
/stuff/linux/arch/riscv/kernel/kgdb.c:39:30: error: use of undeclared identifier 'MASK_BLT'
DECLARE_INSN(blt, MATCH_BLT, MASK_BLT)
                             ^
/stuff/linux/arch/riscv/kernel/kgdb.c:39:19: error: use of undeclared identifier 'MATCH_BLT'
DECLARE_INSN(blt, MATCH_BLT, MASK_BLT)
                  ^
/stuff/linux/arch/riscv/kernel/kgdb.c:40:30: error: use of undeclared identifier 'MASK_BGE'
DECLARE_INSN(bge, MATCH_BGE, MASK_BGE)
                             ^
/stuff/linux/arch/riscv/kernel/kgdb.c:40:19: error: use of undeclared identifier 'MATCH_BGE'
DECLARE_INSN(bge, MATCH_BGE, MASK_BGE)
                  ^
/stuff/linux/arch/riscv/kernel/kgdb.c:41:32: error: use of undeclared identifier 'MASK_BLTU'
DECLARE_INSN(bltu, MATCH_BLTU, MASK_BLTU)
                               ^
fatal error: too many errors emitted, stopping now [-ferror-limit=]
20 errors generated.
make[5]: *** [/stuff/linux/scripts/Makefile.build:250: arch/riscv/kernel/kgdb.o] Error 1

:(
Heiko Stuebner Nov. 30, 2022, 2:04 p.m. UTC | #3 
Am Mittwoch, 30. November 2022, 00:01:10 CET schrieb Conor Dooley:
> Woops, I built this commit in isolation and:
> /stuff/linux/arch/riscv/kernel/kgdb.c:32:32: error: use of undeclared identifier 'MASK_JALR'
> DECLARE_INSN(jalr, MATCH_JALR, MASK_JALR)
>                                ^
> /stuff/linux/arch/riscv/kernel/kgdb.c:32:20: error: use of undeclared identifier 'MATCH_JALR'
> DECLARE_INSN(jalr, MATCH_JALR, MASK_JALR)
>                    ^

actually that is patch1's fault, that added the prefixes in the first place,
and I've fixed it there.
Andrew Jones Nov. 30, 2022, 2:16 p.m. UTC | #4 
On Wed, Nov 30, 2022 at 03:04:44PM +0100, Heiko Stübner wrote:
> Am Mittwoch, 30. November 2022, 00:01:10 CET schrieb Conor Dooley:
> > Woops, I built this commit in isolation and:
> > /stuff/linux/arch/riscv/kernel/kgdb.c:32:32: error: use of undeclared identifier 'MASK_JALR'
> > DECLARE_INSN(jalr, MATCH_JALR, MASK_JALR)
> >                                ^
> > /stuff/linux/arch/riscv/kernel/kgdb.c:32:20: error: use of undeclared identifier 'MATCH_JALR'
> > DECLARE_INSN(jalr, MATCH_JALR, MASK_JALR)
> >                    ^
> 
> actually that is patch1's fault, that added the prefixes in the first place,
> and I've fixed it there.

I guess I should have done my grepping for patch1's review with only it
checked out...

 git rebase -i -x '<build command line>' <series-base-commit>

is our friend.

Thanks,
drew
Heiko Stuebner Nov. 30, 2022, 2:19 p.m. UTC | #5 
Am Mittwoch, 30. November 2022, 15:16:58 CET schrieb Andrew Jones:
> On Wed, Nov 30, 2022 at 03:04:44PM +0100, Heiko Stübner wrote:
> > Am Mittwoch, 30. November 2022, 00:01:10 CET schrieb Conor Dooley:
> > > Woops, I built this commit in isolation and:
> > > /stuff/linux/arch/riscv/kernel/kgdb.c:32:32: error: use of undeclared identifier 'MASK_JALR'
> > > DECLARE_INSN(jalr, MATCH_JALR, MASK_JALR)
> > >                                ^
> > > /stuff/linux/arch/riscv/kernel/kgdb.c:32:20: error: use of undeclared identifier 'MATCH_JALR'
> > > DECLARE_INSN(jalr, MATCH_JALR, MASK_JALR)
> > >                    ^
> > 
> > actually that is patch1's fault, that added the prefixes in the first place,
> > and I've fixed it there.
> 
> I guess I should have done my grepping for patch1's review with only it
> checked out...
> 
>  git rebase -i -x '<build command line>' <series-base-commit>

voodoo :-D

I'm doing that manually right now, I need to address Conor's comments
anyway ;-) .

Heiko
Andrew Jones Nov. 30, 2022, 2:51 p.m. UTC | #6 
On Mon, Nov 28, 2022 at 11:26:21AM +0100, Heiko Stuebner wrote:
> From: Heiko Stuebner <heiko.stuebner@vrull.eu>
> 
> Only carry the actual values for the funct3, funct4, etc instruction
> fields without directly including the shift to the target position.
> 
> Instead use macros to move the values to their target positions
> when needed.
> 
> This at the same time also reduces the use of magic numbers,
> one would need a spec manual to understand.
> 
> Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
> ---
>  arch/riscv/include/asm/parse_asm.h | 100 +++++++++++++++++------------
>  1 file changed, 59 insertions(+), 41 deletions(-)
> 
> diff --git a/arch/riscv/include/asm/parse_asm.h b/arch/riscv/include/asm/parse_asm.h
> index c52b8ae02cfe..e3f87da108f4 100644
> --- a/arch/riscv/include/asm/parse_asm.h
> +++ b/arch/riscv/include/asm/parse_asm.h
> @@ -5,6 +5,15 @@
>  
>  #include <linux/bits.h>
>  
> +#define RV_INSN_FUNCT3_MASK	GENMASK(14, 12)
> +#define RV_INSN_FUNCT3_OPOFF	12
> +#define RV_INSN_OPCODE_MASK	GENMASK(6, 0)
> +#define RV_INSN_OPCODE_OPOFF	0
> +#define RV_INSN_FUNCT12_OPOFF	20
> +
> +#define RV_ENCODE_FUNCT3(f_)	(RVG_FUNCT3_##f_ << RV_INSN_FUNCT3_OPOFF)
> +#define RV_ENCODE_FUNCT12(f_)	(RVG_FUNCT12_##f_ << RV_INSN_FUNCT12_OPOFF)
> +
>  /* The bit field of immediate value in I-type instruction */
>  #define RV_I_IMM_SIGN_OPOFF	31
>  #define RV_I_IMM_11_0_OPOFF	20
> @@ -84,6 +93,15 @@
>  #define RVC_B_IMM_2_1_MASK	GENMASK(1, 0)
>  #define RVC_B_IMM_5_MASK	GENMASK(0, 0)
>  
> +#define RVC_INSN_FUNCT4_MASK	GENMASK(15, 12)
> +#define RVC_INSN_FUNCT4_OPOFF	12
> +#define RVC_INSN_FUNCT3_MASK	GENMASK(15, 13)
> +#define RVC_INSN_FUNCT3_OPOFF	13
> +#define RVC_INSN_J_RS2_MASK	GENMASK(6, 2)
> +#define RVC_INSN_OPCODE_MASK	GENMASK(1, 0)
> +#define RVC_ENCODE_FUNCT3(f_)	(RVC_FUNCT3_##f_ << RVC_INSN_FUNCT3_OPOFF)
> +#define RVC_ENCODE_FUNCT4(f_)	(RVC_FUNCT4_##f_ << RVC_INSN_FUNCT4_OPOFF)
> +
>  /* The register offset in RVC op=C0 instruction */
>  #define RVC_C0_RS1_OPOFF	7
>  #define RVC_C0_RS2_OPOFF	2
> @@ -113,52 +131,52 @@
>  /* parts of funct3 code for I, M, A extension*/
>  #define RVG_FUNCT3_JALR		0x0
>  #define RVG_FUNCT3_BEQ		0x0
> -#define RVG_FUNCT3_BNE		0x1000
> -#define RVG_FUNCT3_BLT		0x4000
> -#define RVG_FUNCT3_BGE		0x5000
> -#define RVG_FUNCT3_BLTU		0x6000
> -#define RVG_FUNCT3_BGEU		0x7000
> +#define RVG_FUNCT3_BNE		0x1
> +#define RVG_FUNCT3_BLT		0x4
> +#define RVG_FUNCT3_BGE		0x5
> +#define RVG_FUNCT3_BLTU		0x6
> +#define RVG_FUNCT3_BGEU		0x7
>  
>  /* parts of funct3 code for C extension*/
> -#define RVC_FUNCT3_C_BEQZ	0xc000
> -#define RVC_FUNCT3_C_BNEZ	0xe000
> -#define RVC_FUNCT3_C_J		0xa000
> -#define RVC_FUNCT3_C_JAL	0x2000
> -#define RVC_FUNCT4_C_JR		0x8000
> -#define RVC_FUNCT4_C_JALR	0xf000
                                ^^ this looks wrong

> +#define RVC_FUNCT3_C_BEQZ	0x6
> +#define RVC_FUNCT3_C_BNEZ	0x7
> +#define RVC_FUNCT3_C_J		0x5
> +#define RVC_FUNCT3_C_JAL	0x1
> +#define RVC_FUNCT4_C_JR		0x8
> +#define RVC_FUNCT4_C_JALR	0x9
                                ^^ and this looks right,
				   but we should fix that with a separate
				   patch

>  
> -#define RVG_FUNCT12_SRET	0x10200000
> +#define RVG_FUNCT12_SRET	0x102
>  
> -#define RVG_MATCH_JALR		(RVG_FUNCT3_JALR | RVG_OPCODE_JALR)
> +#define RVG_MATCH_JALR		(RV_ENCODE_FUNCT3(JALR) | RVG_OPCODE_JALR)
>  #define RVG_MATCH_JAL		(RVG_OPCODE_JAL)
> -#define RVG_MATCH_BEQ		(RVG_FUNCT3_BEQ | RVG_OPCODE_BRANCH)
> -#define RVG_MATCH_BNE		(RVG_FUNCT3_BNE | RVG_OPCODE_BRANCH)
> -#define RVG_MATCH_BLT		(RVG_FUNCT3_BLT | RVG_OPCODE_BRANCH)
> -#define RVG_MATCH_BGE		(RVG_FUNCT3_BGE | RVG_OPCODE_BRANCH)
> -#define RVG_MATCH_BLTU		(RVG_FUNCT3_BLTU | RVG_OPCODE_BRANCH)
> -#define RVG_MATCH_BGEU		(RVG_FUNCT3_BGEU | RVG_OPCODE_BRANCH)
> -#define RVG_MATCH_SRET		(RVG_FUNCT12_SRET | RVG_OPCODE_SYSTEM)
> -#define RVC_MATCH_C_BEQZ	(RVC_FUNCT3_C_BEQZ | RVC_OPCODE_C1)
> -#define RVC_MATCH_C_BNEZ	(RVC_FUNCT3_C_BNEZ | RVC_OPCODE_C1)
> -#define RVC_MATCH_C_J		(RVC_FUNCT3_C_J | RVC_OPCODE_C1)
> -#define RVC_MATCH_C_JAL		(RVC_FUNCT3_C_JAL | RVC_OPCODE_C1)
> -#define RVC_MATCH_C_JR		(RVC_FUNCT4_C_JR | RVC_OPCODE_C2)
> -#define RVC_MATCH_C_JALR	(RVC_FUNCT4_C_JALR | RVC_OPCODE_C2)
> -
> -#define RVG_MASK_JALR		0x707f
> -#define RVG_MASK_JAL		0x7f
> -#define RVC_MASK_C_JALR		0xf07f
> -#define RVC_MASK_C_JR		0xf07f
> -#define RVC_MASK_C_JAL		0xe003
> -#define RVC_MASK_C_J		0xe003
> -#define RVG_MASK_BEQ		0x707f
> -#define RVG_MASK_BNE		0x707f
> -#define RVG_MASK_BLT		0x707f
> -#define RVG_MASK_BGE		0x707f
> -#define RVG_MASK_BLTU		0x707f
> -#define RVG_MASK_BGEU		0x707f
> -#define RVC_MASK_C_BEQZ		0xe003
> -#define RVC_MASK_C_BNEZ		0xe003
> +#define RVG_MATCH_BEQ		(RV_ENCODE_FUNCT3(BEQ) | RVG_OPCODE_BRANCH)
> +#define RVG_MATCH_BNE		(RV_ENCODE_FUNCT3(BNE) | RVG_OPCODE_BRANCH)
> +#define RVG_MATCH_BLT		(RV_ENCODE_FUNCT3(BLT) | RVG_OPCODE_BRANCH)
> +#define RVG_MATCH_BGE		(RV_ENCODE_FUNCT3(BGE) | RVG_OPCODE_BRANCH)
> +#define RVG_MATCH_BLTU		(RV_ENCODE_FUNCT3(BLTU) | RVG_OPCODE_BRANCH)
> +#define RVG_MATCH_BGEU		(RV_ENCODE_FUNCT3(BGEU) | RVG_OPCODE_BRANCH)
> +#define RVG_MATCH_SRET		(RV_ENCODE_FUNCT12(SRET) | RVG_OPCODE_SYSTEM)
> +#define RVC_MATCH_C_BEQZ	(RVC_ENCODE_FUNCT3(C_BEQZ) | RVC_OPCODE_C1)
> +#define RVC_MATCH_C_BNEZ	(RVC_ENCODE_FUNCT3(C_BNEZ) | RVC_OPCODE_C1)
> +#define RVC_MATCH_C_J		(RVC_ENCODE_FUNCT3(C_J) | RVC_OPCODE_C1)
> +#define RVC_MATCH_C_JAL		(RVC_ENCODE_FUNCT3(C_JAL) | RVC_OPCODE_C1)
> +#define RVC_MATCH_C_JR		(RVC_ENCODE_FUNCT4(C_JR) | RVC_OPCODE_C2)
> +#define RVC_MATCH_C_JALR	(RVC_ENCODE_FUNCT4(C_JALR) | RVC_OPCODE_C2)
> +
> +#define RVG_MASK_JALR		(RV_INSN_FUNCT3_MASK | RV_INSN_OPCODE_MASK)
> +#define RVG_MASK_JAL		(RV_INSN_OPCODE_MASK)
> +#define RVC_MASK_C_JALR		(RVC_INSN_FUNCT4_MASK | RVC_INSN_J_RS2_MASK | RVC_INSN_OPCODE_MASK)
> +#define RVC_MASK_C_JR		(RVC_INSN_FUNCT4_MASK | RVC_INSN_J_RS2_MASK | RVC_INSN_OPCODE_MASK)
> +#define RVC_MASK_C_JAL		(RVC_INSN_FUNCT3_MASK | RVC_INSN_OPCODE_MASK)
> +#define RVC_MASK_C_J		(RVC_INSN_FUNCT3_MASK | RVC_INSN_OPCODE_MASK)
> +#define RVG_MASK_BEQ		(RV_INSN_FUNCT3_MASK | RV_INSN_OPCODE_MASK)
> +#define RVG_MASK_BNE		(RV_INSN_FUNCT3_MASK | RV_INSN_OPCODE_MASK)
> +#define RVG_MASK_BLT		(RV_INSN_FUNCT3_MASK | RV_INSN_OPCODE_MASK)
> +#define RVG_MASK_BGE		(RV_INSN_FUNCT3_MASK | RV_INSN_OPCODE_MASK)
> +#define RVG_MASK_BLTU		(RV_INSN_FUNCT3_MASK | RV_INSN_OPCODE_MASK)
> +#define RVG_MASK_BGEU		(RV_INSN_FUNCT3_MASK | RV_INSN_OPCODE_MASK)
> +#define RVC_MASK_C_BEQZ		(RVC_INSN_FUNCT3_MASK | RVC_INSN_OPCODE_MASK)
> +#define RVC_MASK_C_BNEZ		(RVC_INSN_FUNCT3_MASK | RVC_INSN_OPCODE_MASK)
>  #define RVG_MASK_SRET		0xffffffff
>  
>  #define __INSN_LENGTH_MASK	_UL(0x3)
> -- 
> 2.35.1
>

Thanks,
drew
diff mbox series

Patch

diff --git a/arch/riscv/include/asm/parse_asm.h b/arch/riscv/include/asm/parse_asm.h
index c52b8ae02cfe..e3f87da108f4 100644
--- a/arch/riscv/include/asm/parse_asm.h
+++ b/arch/riscv/include/asm/parse_asm.h
@@ -5,6 +5,15 @@ 
 
 #include <linux/bits.h>
 
+#define RV_INSN_FUNCT3_MASK	GENMASK(14, 12)
+#define RV_INSN_FUNCT3_OPOFF	12
+#define RV_INSN_OPCODE_MASK	GENMASK(6, 0)
+#define RV_INSN_OPCODE_OPOFF	0
+#define RV_INSN_FUNCT12_OPOFF	20
+
+#define RV_ENCODE_FUNCT3(f_)	(RVG_FUNCT3_##f_ << RV_INSN_FUNCT3_OPOFF)
+#define RV_ENCODE_FUNCT12(f_)	(RVG_FUNCT12_##f_ << RV_INSN_FUNCT12_OPOFF)
+
 /* The bit field of immediate value in I-type instruction */
 #define RV_I_IMM_SIGN_OPOFF	31
 #define RV_I_IMM_11_0_OPOFF	20
@@ -84,6 +93,15 @@ 
 #define RVC_B_IMM_2_1_MASK	GENMASK(1, 0)
 #define RVC_B_IMM_5_MASK	GENMASK(0, 0)
 
+#define RVC_INSN_FUNCT4_MASK	GENMASK(15, 12)
+#define RVC_INSN_FUNCT4_OPOFF	12
+#define RVC_INSN_FUNCT3_MASK	GENMASK(15, 13)
+#define RVC_INSN_FUNCT3_OPOFF	13
+#define RVC_INSN_J_RS2_MASK	GENMASK(6, 2)
+#define RVC_INSN_OPCODE_MASK	GENMASK(1, 0)
+#define RVC_ENCODE_FUNCT3(f_)	(RVC_FUNCT3_##f_ << RVC_INSN_FUNCT3_OPOFF)
+#define RVC_ENCODE_FUNCT4(f_)	(RVC_FUNCT4_##f_ << RVC_INSN_FUNCT4_OPOFF)
+
 /* The register offset in RVC op=C0 instruction */
 #define RVC_C0_RS1_OPOFF	7
 #define RVC_C0_RS2_OPOFF	2
@@ -113,52 +131,52 @@ 
 /* parts of funct3 code for I, M, A extension*/
 #define RVG_FUNCT3_JALR		0x0
 #define RVG_FUNCT3_BEQ		0x0
-#define RVG_FUNCT3_BNE		0x1000
-#define RVG_FUNCT3_BLT		0x4000
-#define RVG_FUNCT3_BGE		0x5000
-#define RVG_FUNCT3_BLTU		0x6000
-#define RVG_FUNCT3_BGEU		0x7000
+#define RVG_FUNCT3_BNE		0x1
+#define RVG_FUNCT3_BLT		0x4
+#define RVG_FUNCT3_BGE		0x5
+#define RVG_FUNCT3_BLTU		0x6
+#define RVG_FUNCT3_BGEU		0x7
 
 /* parts of funct3 code for C extension*/
-#define RVC_FUNCT3_C_BEQZ	0xc000
-#define RVC_FUNCT3_C_BNEZ	0xe000
-#define RVC_FUNCT3_C_J		0xa000
-#define RVC_FUNCT3_C_JAL	0x2000
-#define RVC_FUNCT4_C_JR		0x8000
-#define RVC_FUNCT4_C_JALR	0xf000
+#define RVC_FUNCT3_C_BEQZ	0x6
+#define RVC_FUNCT3_C_BNEZ	0x7
+#define RVC_FUNCT3_C_J		0x5
+#define RVC_FUNCT3_C_JAL	0x1
+#define RVC_FUNCT4_C_JR		0x8
+#define RVC_FUNCT4_C_JALR	0x9
 
-#define RVG_FUNCT12_SRET	0x10200000
+#define RVG_FUNCT12_SRET	0x102
 
-#define RVG_MATCH_JALR		(RVG_FUNCT3_JALR | RVG_OPCODE_JALR)
+#define RVG_MATCH_JALR		(RV_ENCODE_FUNCT3(JALR) | RVG_OPCODE_JALR)
 #define RVG_MATCH_JAL		(RVG_OPCODE_JAL)
-#define RVG_MATCH_BEQ		(RVG_FUNCT3_BEQ | RVG_OPCODE_BRANCH)
-#define RVG_MATCH_BNE		(RVG_FUNCT3_BNE | RVG_OPCODE_BRANCH)
-#define RVG_MATCH_BLT		(RVG_FUNCT3_BLT | RVG_OPCODE_BRANCH)
-#define RVG_MATCH_BGE		(RVG_FUNCT3_BGE | RVG_OPCODE_BRANCH)
-#define RVG_MATCH_BLTU		(RVG_FUNCT3_BLTU | RVG_OPCODE_BRANCH)
-#define RVG_MATCH_BGEU		(RVG_FUNCT3_BGEU | RVG_OPCODE_BRANCH)
-#define RVG_MATCH_SRET		(RVG_FUNCT12_SRET | RVG_OPCODE_SYSTEM)
-#define RVC_MATCH_C_BEQZ	(RVC_FUNCT3_C_BEQZ | RVC_OPCODE_C1)
-#define RVC_MATCH_C_BNEZ	(RVC_FUNCT3_C_BNEZ | RVC_OPCODE_C1)
-#define RVC_MATCH_C_J		(RVC_FUNCT3_C_J | RVC_OPCODE_C1)
-#define RVC_MATCH_C_JAL		(RVC_FUNCT3_C_JAL | RVC_OPCODE_C1)
-#define RVC_MATCH_C_JR		(RVC_FUNCT4_C_JR | RVC_OPCODE_C2)
-#define RVC_MATCH_C_JALR	(RVC_FUNCT4_C_JALR | RVC_OPCODE_C2)
-
-#define RVG_MASK_JALR		0x707f
-#define RVG_MASK_JAL		0x7f
-#define RVC_MASK_C_JALR		0xf07f
-#define RVC_MASK_C_JR		0xf07f
-#define RVC_MASK_C_JAL		0xe003
-#define RVC_MASK_C_J		0xe003
-#define RVG_MASK_BEQ		0x707f
-#define RVG_MASK_BNE		0x707f
-#define RVG_MASK_BLT		0x707f
-#define RVG_MASK_BGE		0x707f
-#define RVG_MASK_BLTU		0x707f
-#define RVG_MASK_BGEU		0x707f
-#define RVC_MASK_C_BEQZ		0xe003
-#define RVC_MASK_C_BNEZ		0xe003
+#define RVG_MATCH_BEQ		(RV_ENCODE_FUNCT3(BEQ) | RVG_OPCODE_BRANCH)
+#define RVG_MATCH_BNE		(RV_ENCODE_FUNCT3(BNE) | RVG_OPCODE_BRANCH)
+#define RVG_MATCH_BLT		(RV_ENCODE_FUNCT3(BLT) | RVG_OPCODE_BRANCH)
+#define RVG_MATCH_BGE		(RV_ENCODE_FUNCT3(BGE) | RVG_OPCODE_BRANCH)
+#define RVG_MATCH_BLTU		(RV_ENCODE_FUNCT3(BLTU) | RVG_OPCODE_BRANCH)
+#define RVG_MATCH_BGEU		(RV_ENCODE_FUNCT3(BGEU) | RVG_OPCODE_BRANCH)
+#define RVG_MATCH_SRET		(RV_ENCODE_FUNCT12(SRET) | RVG_OPCODE_SYSTEM)
+#define RVC_MATCH_C_BEQZ	(RVC_ENCODE_FUNCT3(C_BEQZ) | RVC_OPCODE_C1)
+#define RVC_MATCH_C_BNEZ	(RVC_ENCODE_FUNCT3(C_BNEZ) | RVC_OPCODE_C1)
+#define RVC_MATCH_C_J		(RVC_ENCODE_FUNCT3(C_J) | RVC_OPCODE_C1)
+#define RVC_MATCH_C_JAL		(RVC_ENCODE_FUNCT3(C_JAL) | RVC_OPCODE_C1)
+#define RVC_MATCH_C_JR		(RVC_ENCODE_FUNCT4(C_JR) | RVC_OPCODE_C2)
+#define RVC_MATCH_C_JALR	(RVC_ENCODE_FUNCT4(C_JALR) | RVC_OPCODE_C2)
+
+#define RVG_MASK_JALR		(RV_INSN_FUNCT3_MASK | RV_INSN_OPCODE_MASK)
+#define RVG_MASK_JAL		(RV_INSN_OPCODE_MASK)
+#define RVC_MASK_C_JALR		(RVC_INSN_FUNCT4_MASK | RVC_INSN_J_RS2_MASK | RVC_INSN_OPCODE_MASK)
+#define RVC_MASK_C_JR		(RVC_INSN_FUNCT4_MASK | RVC_INSN_J_RS2_MASK | RVC_INSN_OPCODE_MASK)
+#define RVC_MASK_C_JAL		(RVC_INSN_FUNCT3_MASK | RVC_INSN_OPCODE_MASK)
+#define RVC_MASK_C_J		(RVC_INSN_FUNCT3_MASK | RVC_INSN_OPCODE_MASK)
+#define RVG_MASK_BEQ		(RV_INSN_FUNCT3_MASK | RV_INSN_OPCODE_MASK)
+#define RVG_MASK_BNE		(RV_INSN_FUNCT3_MASK | RV_INSN_OPCODE_MASK)
+#define RVG_MASK_BLT		(RV_INSN_FUNCT3_MASK | RV_INSN_OPCODE_MASK)
+#define RVG_MASK_BGE		(RV_INSN_FUNCT3_MASK | RV_INSN_OPCODE_MASK)
+#define RVG_MASK_BLTU		(RV_INSN_FUNCT3_MASK | RV_INSN_OPCODE_MASK)
+#define RVG_MASK_BGEU		(RV_INSN_FUNCT3_MASK | RV_INSN_OPCODE_MASK)
+#define RVC_MASK_C_BEQZ		(RVC_INSN_FUNCT3_MASK | RVC_INSN_OPCODE_MASK)
+#define RVC_MASK_C_BNEZ		(RVC_INSN_FUNCT3_MASK | RVC_INSN_OPCODE_MASK)
 #define RVG_MASK_SRET		0xffffffff
 
 #define __INSN_LENGTH_MASK	_UL(0x3)