| Message ID | 1660013580-19053-2-git-send-email-yangtiezhu@loongson.cn (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add BPF JIT support for LoongArch | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch, async |
| bpf/vmtest-bpf-PR | fail | merge-conflict |
Hi, Tiezhu On 08/09/2022 10:52 AM, Tiezhu Yang wrote: > struct reg2i12_format is used to generate the instruction lu52id > in larch_insn_gen_lu52id(), according to the instruction format > of lu52id in LoongArch Reference Manual [1], the type of field > "immediate" should be "signed int" rather than "unsigned int". > > There are similar problems in the other structs reg0i26_format, > reg1i20_format, reg1i21_format and reg2i16_format, fix them. > > [1] https://loongson.github.io/LoongArch-Documentation/LoongArch-Vol1-EN.html#_lu12i_w_lu32i_d_lu52i_d > > Fixes: b738c106f735 ("LoongArch: Add other common headers") > We may not be able to say "Fixes" here, because it is also correct to treat each field of the instruction as an "unsinged int" type (signed or not has no effect on the machine instruction stream, but it does affect the programmer). For example, when reg2i12_format.immediate is changed to "signed" type, the immediate judgment in is_stack_alloc_ins() can be simplified, static inline bool is_stack_alloc_ins(union loongarch_instruction *ip) { /* addi.d $sp, $sp, -imm */ return ip->reg2i12_format.opcode == addid_op && ip->reg2i12_format.rj == LOONGARCH_GPR_SP && ip->reg2i12_format.rd == LOONGARCH_GPR_SP && - is_imm12_negative(ip->reg2i12_format.immediate); + (ip->reg2i12_format.immediate < 0; } Thanks, Youling > Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn> > --- > arch/loongarch/include/asm/inst.h | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/arch/loongarch/include/asm/inst.h b/arch/loongarch/include/asm/inst.h > index 7b07cbb..ff51481 100644 > --- a/arch/loongarch/include/asm/inst.h > +++ b/arch/loongarch/include/asm/inst.h > @@ -53,35 +53,35 @@ enum reg2i16_op { > }; > > struct reg0i26_format { > - unsigned int immediate_h : 10; > - unsigned int immediate_l : 16; > + signed int immediate_h : 10; > + signed int immediate_l : 16; > unsigned int opcode : 6; > }; > > struct reg1i20_format { > unsigned int rd : 5; > - unsigned int immediate : 20; > + signed int immediate : 20; > unsigned int opcode : 7; > }; > > struct reg1i21_format { > - unsigned int immediate_h : 5; > + signed int immediate_h : 5; > unsigned int rj : 5; > - unsigned int immediate_l : 16; > + signed int immediate_l : 16; > unsigned int opcode : 6; > }; > > struct reg2i12_format { > unsigned int rd : 5; > unsigned int rj : 5; > - unsigned int immediate : 12; > + signed int immediate : 12; > unsigned int opcode : 10; > }; > > struct reg2i16_format { > unsigned int rd : 5; > unsigned int rj : 5; > - unsigned int immediate : 16; > + signed int immediate : 16; > unsigned int opcode : 6; > }; > >
On Tue, Aug 9, 2022 at 8:01 PM Youling Tang <tangyouling@loongson.cn> wrote: > > Hi, Tiezhu > > On 08/09/2022 10:52 AM, Tiezhu Yang wrote: > > struct reg2i12_format is used to generate the instruction lu52id > > in larch_insn_gen_lu52id(), according to the instruction format > > of lu52id in LoongArch Reference Manual [1], the type of field > > "immediate" should be "signed int" rather than "unsigned int". > > > > There are similar problems in the other structs reg0i26_format, > > reg1i20_format, reg1i21_format and reg2i16_format, fix them. > > > > [1] https://loongson.github.io/LoongArch-Documentation/LoongArch-Vol1-EN.html#_lu12i_w_lu32i_d_lu52i_d > > > > Fixes: b738c106f735 ("LoongArch: Add other common headers") > > > We may not be able to say "Fixes" here, because it is also correct to > treat each field of the instruction as an "unsinged int" type (signed > or not has no effect on the machine instruction stream, but it does > affect the programmer). > > For example, when reg2i12_format.immediate is changed to "signed" type, > the immediate judgment in is_stack_alloc_ins() can be simplified, I prefer to use "unsigned int" because in an instruction the imm field is essentially just bit-stream. Huacai > > static inline bool is_stack_alloc_ins(union loongarch_instruction *ip) > { > /* addi.d $sp, $sp, -imm */ > return ip->reg2i12_format.opcode == addid_op && > ip->reg2i12_format.rj == LOONGARCH_GPR_SP && > ip->reg2i12_format.rd == LOONGARCH_GPR_SP && > - is_imm12_negative(ip->reg2i12_format.immediate); > + (ip->reg2i12_format.immediate < 0; > } > > > Thanks, > Youling > > Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn> > > --- > > arch/loongarch/include/asm/inst.h | 14 +++++++------- > > 1 file changed, 7 insertions(+), 7 deletions(-) > > > > diff --git a/arch/loongarch/include/asm/inst.h b/arch/loongarch/include/asm/inst.h > > index 7b07cbb..ff51481 100644 > > --- a/arch/loongarch/include/asm/inst.h > > +++ b/arch/loongarch/include/asm/inst.h > > @@ -53,35 +53,35 @@ enum reg2i16_op { > > }; > > > > struct reg0i26_format { > > - unsigned int immediate_h : 10; > > - unsigned int immediate_l : 16; > > + signed int immediate_h : 10; > > + signed int immediate_l : 16; > > unsigned int opcode : 6; > > }; > > > > struct reg1i20_format { > > unsigned int rd : 5; > > - unsigned int immediate : 20; > > + signed int immediate : 20; > > unsigned int opcode : 7; > > }; > > > > struct reg1i21_format { > > - unsigned int immediate_h : 5; > > + signed int immediate_h : 5; > > unsigned int rj : 5; > > - unsigned int immediate_l : 16; > > + signed int immediate_l : 16; > > unsigned int opcode : 6; > > }; > > > > struct reg2i12_format { > > unsigned int rd : 5; > > unsigned int rj : 5; > > - unsigned int immediate : 12; > > + signed int immediate : 12; > > unsigned int opcode : 10; > > }; > > > > struct reg2i16_format { > > unsigned int rd : 5; > > unsigned int rj : 5; > > - unsigned int immediate : 16; > > + signed int immediate : 16; > > unsigned int opcode : 6; > > }; > > > > >
diff --git a/arch/loongarch/include/asm/inst.h b/arch/loongarch/include/asm/inst.h index 7b07cbb..ff51481 100644 --- a/arch/loongarch/include/asm/inst.h +++ b/arch/loongarch/include/asm/inst.h @@ -53,35 +53,35 @@ enum reg2i16_op { }; struct reg0i26_format { - unsigned int immediate_h : 10; - unsigned int immediate_l : 16; + signed int immediate_h : 10; + signed int immediate_l : 16; unsigned int opcode : 6; }; struct reg1i20_format { unsigned int rd : 5; - unsigned int immediate : 20; + signed int immediate : 20; unsigned int opcode : 7; }; struct reg1i21_format { - unsigned int immediate_h : 5; + signed int immediate_h : 5; unsigned int rj : 5; - unsigned int immediate_l : 16; + signed int immediate_l : 16; unsigned int opcode : 6; }; struct reg2i12_format { unsigned int rd : 5; unsigned int rj : 5; - unsigned int immediate : 12; + signed int immediate : 12; unsigned int opcode : 10; }; struct reg2i16_format { unsigned int rd : 5; unsigned int rj : 5; - unsigned int immediate : 16; + signed int immediate : 16; unsigned int opcode : 6; };
struct reg2i12_format is used to generate the instruction lu52id in larch_insn_gen_lu52id(), according to the instruction format of lu52id in LoongArch Reference Manual [1], the type of field "immediate" should be "signed int" rather than "unsigned int". There are similar problems in the other structs reg0i26_format, reg1i20_format, reg1i21_format and reg2i16_format, fix them. [1] https://loongson.github.io/LoongArch-Documentation/LoongArch-Vol1-EN.html#_lu12i_w_lu32i_d_lu52i_d Fixes: b738c106f735 ("LoongArch: Add other common headers") Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn> --- arch/loongarch/include/asm/inst.h | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)