Hi Jinyang,
Thank you very much for your review.
On 08/22/2022 10:50 AM, Jinyang He wrote:
Hi, Tiezhu,
[...]
+ emit_insn(ctx, addid, LOONGARCH_GPR_SP, LOONGARCH_GPR_SP,
-stack_adjust);
Have you checked the stack size before this, such as in compiler or
common codes? Is there any chance of overflow 12bits range?
MAX_BPF_STACK is 512, BPF program can access up to 512 bytes of stack
space, so it is OK here.
+ emit_insn(ctx, sllid, t2, a2, 3);
+ emit_insn(ctx, addd, t2, t2, a1);
alsl.d
[...]
+ /* zero-extend 16 bits into 64 bits */
+ emit_insn(ctx, sllid, dst, dst, 48);
+ emit_insn(ctx, srlid, dst, dst, 48);
bstrpick.d
[...]
+ move_imm32(ctx, t1, imm, false);
imm = 0 -> t1->zero
[...]
In BFD_W, BFF_DW cases, if offsets is quadruple and in 16bits range,
we can use [ld/st]ptr.[w/d].
Good catch, I will modify the related code to save some instructions.
+static inline void emit_cond_jmp(struct jit_ctx *ctx, u8 cond, enum
loongarch_gpr rj,
+ enum loongarch_gpr rd, int jmp_offset)
+{
+ cond_jmp_offs26(ctx, cond, rj, rd, jmp_offset);
Why not call cond_jmp_offs16 as a preference?
Good question, this is intended to handle some special test cases.
A large PC-relative jump offset may overflow the immediate field of
the native conditional branch instruction, triggering a conversion
to use an absolute jump instead, this jump sequence is particularly
nasty. For now, let us use cond_jmp_offs26() directly to keep it
simple. In the future, maybe we can add support for far branching,
the branch relaxation requires more than two passes to converge,
the code seems too complex to understand, not quite sure whether it
is necessary and worth the extra pain. Anyway, just leave it as is
to enhance code readability now.
I will add code comments to explain the above considerations.
Thanks,
Tiezhu
