Skip to content

Commit 326e114

Browse files
agattidpgeorge
authored andcommittedAug 19, 2024
py/asmrv32: Fix short/long jumps scheduling.
The RV32 emitter always scheduled short jumps even outside the emit compiler pass. Running the full test suite through the native emitter instead of just the tests that depend on the emitter at runtime (as in, `micropython/native_*` and `micropython/viper_* tests`) uncovered more places where the invalid behaviour was still present. Signed-off-by: Alessandro Gatti <[email protected]>
1 parent 6367099 commit 326e114

File tree

2 files changed

+39
-34
lines changed

2 files changed

+39
-34
lines changed
 

‎py/asmrv32.c

+37-32
Original file line numberDiff line numberDiff line change
@@ -132,9 +132,9 @@ static void split_immediate(mp_int_t immediate, mp_uint_t *upper, mp_uint_t *low
132132

133133
static void load_upper_immediate(asm_rv32_t *state, mp_uint_t rd, mp_uint_t immediate) {
134134
// if immediate fits in 17 bits and is ≠ 0:
135-
// c.lui rd, HI(immediate)
135+
// c.lui rd, HI(immediate)
136136
// else:
137-
// lui rd, HI(immediate)
137+
// lui rd, HI(immediate)
138138
if (FIT_SIGNED(immediate, 17) && ((immediate >> 12) != 0)) {
139139
asm_rv32_opcode_clui(state, rd, immediate);
140140
} else {
@@ -270,6 +270,14 @@ static void emit_function_epilogue(asm_rv32_t *state, mp_uint_t registers) {
270270
state->saved_registers_mask = old_saved_registers_mask;
271271
}
272272

273+
static bool calculate_displacement_for_label(asm_rv32_t *state, mp_uint_t label, ptrdiff_t *displacement) {
274+
assert(displacement != NULL && "Displacement pointer is NULL");
275+
276+
mp_uint_t label_offset = state->base.label_offsets[label];
277+
*displacement = (ptrdiff_t)(label_offset - state->base.code_offset);
278+
return (label_offset != (mp_uint_t)-1) && (*displacement < 0);
279+
}
280+
273281
///////////////////////////////////////////////////////////////////////////////
274282

275283
void asm_rv32_entry(asm_rv32_t *state, mp_uint_t locals) {
@@ -326,10 +334,10 @@ void asm_rv32_emit_call_ind(asm_rv32_t *state, mp_uint_t index) {
326334
}
327335

328336
void asm_rv32_emit_jump_if_reg_eq(asm_rv32_t *state, mp_uint_t rs1, mp_uint_t rs2, mp_uint_t label) {
329-
ptrdiff_t displacement = (ptrdiff_t)(state->base.label_offsets[label] - state->base.code_offset);
337+
ptrdiff_t displacement = 0;
338+
bool can_emit_short_jump = calculate_displacement_for_label(state, label, &displacement);
330339

331-
// The least significant bit is ignored anyway.
332-
if (FIT_SIGNED(displacement, 13)) {
340+
if (can_emit_short_jump && FIT_SIGNED(displacement, 13)) {
333341
// beq rs1, rs2, displacement
334342
asm_rv32_opcode_beq(state, rs1, rs2, displacement);
335343
return;
@@ -354,31 +362,24 @@ void asm_rv32_emit_jump_if_reg_eq(asm_rv32_t *state, mp_uint_t rs1, mp_uint_t rs
354362
}
355363

356364
void asm_rv32_emit_jump_if_reg_nonzero(asm_rv32_t *state, mp_uint_t rs, mp_uint_t label) {
357-
ptrdiff_t displacement = (ptrdiff_t)(state->base.label_offsets[label] - state->base.code_offset);
365+
ptrdiff_t displacement = 0;
366+
bool can_emit_short_jump = calculate_displacement_for_label(state, label, &displacement);
358367

359-
if (FIT_SIGNED(displacement, 9) && IS_IN_C_REGISTER_WINDOW(rs)) {
368+
if (can_emit_short_jump && FIT_SIGNED(displacement, 8) && IS_IN_C_REGISTER_WINDOW(rs)) {
360369
// c.bnez rs', displacement
361370
asm_rv32_opcode_cbnez(state, MAP_IN_C_REGISTER_WINDOW(rs), displacement);
362371
return;
363372
}
364373

365-
// The least significant bit is ignored anyway.
366-
if (FIT_SIGNED(displacement, 13)) {
374+
if (can_emit_short_jump && FIT_SIGNED(displacement, 13)) {
367375
// bne rs, zero, displacement
368376
asm_rv32_opcode_bne(state, rs, ASM_RV32_REG_ZERO, displacement);
369377
return;
370378
}
371379

372-
// Compensate for the initial C.BEQZ/BEQ opcode.
373-
displacement -= IS_IN_C_REGISTER_WINDOW(rs) ? ASM_HALFWORD_SIZE : ASM_WORD_SIZE;
374-
375-
mp_uint_t upper = 0;
376-
mp_uint_t lower = 0;
377-
split_immediate(displacement, &upper, &lower);
378-
379380
// TODO: Can this clobber REG_TEMP[0:2]?
380381

381-
// if rs1 in C window (the offset always fits):
382+
// if rs1 in C window and displacement is negative:
382383
// c.beqz rs', 10 ; PC + 0
383384
// auipc temporary, HI(displacement) ; PC + 2
384385
// jalr zero, temporary, LO(displacement) ; PC + 6
@@ -388,11 +389,20 @@ void asm_rv32_emit_jump_if_reg_nonzero(asm_rv32_t *state, mp_uint_t rs, mp_uint_
388389
// auipc temporary, HI(displacement) ; PC + 4
389390
// jalr zero, temporary, LO(displacement) ; PC + 8
390391
// ... ; PC + 12
391-
if (IS_IN_C_REGISTER_WINDOW(rs)) {
392+
393+
if (can_emit_short_jump && IS_IN_C_REGISTER_WINDOW(rs)) {
392394
asm_rv32_opcode_cbeqz(state, MAP_IN_C_REGISTER_WINDOW(rs), 10);
395+
// Compensate for the C.BEQZ opcode.
396+
displacement -= ASM_HALFWORD_SIZE;
393397
} else {
394398
asm_rv32_opcode_beq(state, rs, ASM_RV32_REG_ZERO, 12);
399+
// Compensate for the BEQ opcode.
400+
displacement -= ASM_WORD_SIZE;
395401
}
402+
403+
mp_uint_t upper = 0;
404+
mp_uint_t lower = 0;
405+
split_immediate(displacement, &upper, &lower);
396406
asm_rv32_opcode_auipc(state, INTERNAL_TEMPORARY, upper);
397407
asm_rv32_opcode_jalr(state, ASM_RV32_REG_ZERO, INTERNAL_TEMPORARY, lower);
398408
}
@@ -502,10 +512,10 @@ void asm_rv32_emit_load_reg_reg_offset(asm_rv32_t *state, mp_uint_t rd, mp_uint_
502512
}
503513

504514
void asm_rv32_emit_jump(asm_rv32_t *state, mp_uint_t label) {
505-
ptrdiff_t displacement = (ptrdiff_t)(state->base.label_offsets[label] - state->base.code_offset);
515+
ptrdiff_t displacement = 0;
516+
bool can_emit_short_jump = calculate_displacement_for_label(state, label, &displacement);
506517

507-
// The least significant bit is ignored anyway.
508-
if (FIT_SIGNED(displacement, 13)) {
518+
if (can_emit_short_jump && FIT_SIGNED(displacement, 12)) {
509519
// c.j displacement
510520
asm_rv32_opcode_cj(state, displacement);
511521
return;
@@ -536,12 +546,12 @@ void asm_rv32_emit_store_reg_reg_offset(asm_rv32_t *state, mp_uint_t rd, mp_uint
536546
mp_uint_t lower = 0;
537547
split_immediate(scaled_offset, &upper, &lower);
538548

539-
// lui rd, HI(offset) ; Or c.lui if possible
540-
// c.add rd, rs
541-
// sw rd, LO(offset)(rd)
542-
load_upper_immediate(state, rd, upper);
543-
asm_rv32_opcode_cadd(state, rd, rs);
544-
asm_rv32_opcode_sw(state, rd, rd, lower);
549+
// lui temporary, HI(offset) ; Or c.lui if possible
550+
// c.add temporary, rs
551+
// sw rd, LO(offset)(temporary)
552+
load_upper_immediate(state, INTERNAL_TEMPORARY, upper);
553+
asm_rv32_opcode_cadd(state, INTERNAL_TEMPORARY, rs);
554+
asm_rv32_opcode_sw(state, rd, INTERNAL_TEMPORARY, lower);
545555
}
546556

547557
void asm_rv32_emit_mov_reg_pcrel(asm_rv32_t *state, mp_uint_t rd, mp_uint_t label) {
@@ -550,11 +560,6 @@ void asm_rv32_emit_mov_reg_pcrel(asm_rv32_t *state, mp_uint_t rd, mp_uint_t labe
550560
mp_uint_t lower = 0;
551561
split_immediate(displacement, &upper, &lower);
552562

553-
// Compressed instructions are not used even if they may allow for code size
554-
// savings as the code changes size between compute and emit passes
555-
// otherwise. If that happens then the assertion at asmbase.c:93 triggers
556-
// when built in debug mode.
557-
558563
// auipc rd, HI(relative)
559564
// addi rd, rd, LO(relative)
560565
asm_rv32_opcode_auipc(state, rd, upper);

‎py/asmrv32.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -331,7 +331,7 @@ static inline void asm_rv32_opcode_lw(asm_rv32_t *state, mp_uint_t rd, mp_uint_t
331331
}
332332

333333
// MUL RD, RS1, RS2
334-
static inline void asm_rv32m_opcode_mul(asm_rv32_t *state, mp_uint_t rd, mp_uint_t rs1, mp_uint_t rs2) {
334+
static inline void asm_rv32_opcode_mul(asm_rv32_t *state, mp_uint_t rd, mp_uint_t rs1, mp_uint_t rs2) {
335335
// R: 0000001 ..... ..... 000 ..... 0110011
336336
asm_rv32_emit_word_opcode(state, RV32_ENCODE_TYPE_R(0x33, 0x00, 0x01, rd, rs1, rs2));
337337
}
@@ -479,7 +479,7 @@ void asm_rv32_emit_store_reg_reg_offset(asm_rv32_t *state, mp_uint_t source, mp_
479479
#define ASM_MOV_REG_LOCAL(state, rd, local) asm_rv32_emit_mov_reg_local(state, rd, local)
480480
#define ASM_MOV_REG_PCREL(state, rd, label) asm_rv32_emit_mov_reg_pcrel(state, rd, label)
481481
#define ASM_MOV_REG_REG(state, rd, rs) asm_rv32_opcode_cmv(state, rd, rs)
482-
#define ASM_MUL_REG_REG(state, rd, rs) asm_rv32m_opcode_mul(state, rd, rd, rs)
482+
#define ASM_MUL_REG_REG(state, rd, rs) asm_rv32_opcode_mul(state, rd, rd, rs)
483483
#define ASM_NEG_REG(state, rd) asm_rv32_opcode_sub(state, rd, ASM_RV32_REG_ZERO, rd)
484484
#define ASM_NOT_REG(state, rd) asm_rv32_opcode_xori(state, rd, rd, -1)
485485
#define ASM_OR_REG_REG(state, rd, rs) asm_rv32_opcode_or(state, rd, rd, rs)

0 commit comments

Comments
 (0)
Please sign in to comment.