Skip to content

Commit 2852a99

Browse files
committed
Fix T2C race conditions with immutable cache
The tier-2 JIT compiler (T2C) had critical race conditions, and multiple threads were accessing and modifying shared block structures without proper synchronization. Root causes identified: - T2C compilation thread held pointers to blocks that could be evicted - Block reuse from memory pool caused stale function pointer access - Complex synchronization with hot2 flag and reference counting was insufficient Solution implemented: 1. Created separate immutable T2C cache (t2c_cache) for compiled blocks - T2C entries are never modified once created - Eliminates all race conditions on compiled function access 2. Deep copy block IR chains when queuing for compilation - Compilation thread works on copies, not live blocks - Blocks can be safely evicted during compilation 3. Simplified synchronization logic - Removed hot2 flag and complex reference counting - Use simple compiled flag only to prevent re-queuing - Clear flag when T2C code becomes available 4. Fixed eviction logic to properly check compilation status
1 parent d22b787 commit 2852a99

File tree

5 files changed

+263
-49
lines changed

5 files changed

+263
-49
lines changed

src/emulate.c

Lines changed: 153 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
*/
55

66
#include <assert.h>
7+
#include <sched.h>
8+
#include <stdatomic.h>
79
#include <stdbool.h>
810
#include <stdint.h>
911
#include <stdio.h>
@@ -278,12 +280,13 @@ static block_t *block_alloc(riscv_t *rv)
278280
#if RV32_HAS(JIT)
279281
block->translatable = true;
280282
block->hot = false;
281-
block->hot2 = false;
282283
block->has_loops = false;
283284
block->n_invoke = 0;
284285
INIT_LIST_HEAD(&block->list);
285286
#if RV32_HAS(T2C)
286-
block->compiled = false;
287+
/* Initialize T2C compilation flag */
288+
atomic_store_explicit(&block->compiled, false, memory_order_relaxed);
289+
atomic_store_explicit(&block->func, NULL, memory_order_relaxed);
287290
#endif
288291
#endif
289292
return block;
@@ -918,6 +921,29 @@ static block_t *block_find_or_translate(riscv_t *rv)
918921
return next_blk;
919922
}
920923

924+
#if RV32_HAS(T2C)
925+
/* Avoid evicting blocks being compiled */
926+
if (atomic_load_explicit(&replaced_blk->compiled, memory_order_acquire)) {
927+
/* This block is being compiled - do not evict it.
928+
* Return NULL to signal cache is full.
929+
*/
930+
pthread_mutex_unlock(&rv->cache_lock);
931+
932+
/* Free the newly translated block */
933+
for (rv_insn_t *ir = next_blk->ir_head, *next_ir; ir; ir = next_ir) {
934+
next_ir = ir->next;
935+
free(ir->fuse);
936+
mpool_free(rv->block_ir_mp, ir);
937+
}
938+
list_del_init(&next_blk->list);
939+
atomic_store_explicit(&next_blk->func, NULL, memory_order_relaxed);
940+
mpool_free(rv->block_mp, next_blk);
941+
942+
return NULL;
943+
}
944+
/* Allow evicting blocks that are not yet compiled */
945+
#endif
946+
921947
if (prev == replaced_blk)
922948
prev = NULL;
923949

@@ -930,43 +956,60 @@ static block_t *block_find_or_translate(riscv_t *rv)
930956
rv_insn_t *taken = entry->ir_tail->branch_taken,
931957
*untaken = entry->ir_tail->branch_untaken;
932958

933-
if (taken == replaced_blk_entry) {
959+
if (taken == replaced_blk_entry)
934960
entry->ir_tail->branch_taken = NULL;
935-
}
936-
if (untaken == replaced_blk_entry) {
961+
if (untaken == replaced_blk_entry)
937962
entry->ir_tail->branch_untaken = NULL;
938-
}
939963

940964
/* upadte JALR LUT */
941-
if (!entry->ir_tail->branch_table) {
965+
if (!entry->ir_tail->branch_table)
942966
continue;
943-
}
944967

945-
/**
946-
* TODO: upadate all JALR instructions which references to this
947-
* basic block as the destination.
968+
/* TODO: upadate all JALR instructions which references to this basic
969+
* block as the destination.
948970
*/
949971
}
950972

951-
/* free IRs in replaced block */
952-
for (rv_insn_t *ir = replaced_blk->ir_head, *next_ir; ir != NULL;
953-
ir = next_ir) {
954-
next_ir = ir->next;
973+
#if RV32_HAS(T2C)
974+
/* Double-check - do not evict if being compiled */
975+
if (atomic_load_explicit(&replaced_blk->compiled, memory_order_acquire)) {
976+
/* Block is being compiled - cannot evict */
977+
pthread_mutex_unlock(&rv->cache_lock);
955978

956-
if (ir->fuse)
979+
/* Free the newly translated block */
980+
for (rv_insn_t *ir = next_blk->ir_head, *next_ir; ir; ir = next_ir) {
981+
next_ir = ir->next;
957982
free(ir->fuse);
983+
mpool_free(rv->block_ir_mp, ir);
984+
}
985+
list_del_init(&next_blk->list);
986+
atomic_store_explicit(&next_blk->func, NULL, memory_order_relaxed);
987+
mpool_free(rv->block_mp, next_blk);
958988

989+
return NULL;
990+
}
991+
#endif
992+
/* At this point, block is not compiled - safe to evict */
993+
/* Free the replaced block
994+
*/
995+
for (rv_insn_t *ir = replaced_blk->ir_head, *next_ir; ir; ir = next_ir) {
996+
next_ir = ir->next;
997+
free(ir->fuse);
959998
mpool_free(rv->block_ir_mp, ir);
960999
}
9611000

9621001
list_del_init(&replaced_blk->list);
1002+
#if RV32_HAS(T2C)
1003+
/* Clear atomic fields before returning to pool */
1004+
atomic_store_explicit(&replaced_blk->func, NULL, memory_order_relaxed);
1005+
atomic_store_explicit(&replaced_blk->compiled, false, memory_order_relaxed);
1006+
#endif
9631007
mpool_free(rv->block_mp, replaced_blk);
9641008
#if RV32_HAS(T2C)
9651009
pthread_mutex_unlock(&rv->cache_lock);
9661010
#endif
9671011
#endif
9681012

969-
assert(next_blk);
9701013
return next_blk;
9711014
}
9721015

@@ -1077,6 +1120,15 @@ void rv_step(void *arg)
10771120
* and move onto the next block.
10781121
*/
10791122
block_t *block = block_find_or_translate(rv);
1123+
#if RV32_HAS(T2C)
1124+
if (!block) {
1125+
/* Cache is full of compiled blocks.
1126+
* Try again after yielding to allow T2C thread to complete.
1127+
*/
1128+
sched_yield();
1129+
continue;
1130+
}
1131+
#endif
10801132
/* by now, a block should be available */
10811133
assert(block);
10821134

@@ -1120,20 +1172,88 @@ void rv_step(void *arg)
11201172
last_pc = rv->PC;
11211173
#if RV32_HAS(JIT)
11221174
#if RV32_HAS(T2C)
1123-
/* executed through the tier-2 JIT compiler */
1124-
if (block->hot2) {
1125-
((exec_t2c_func_t) block->func)(rv);
1175+
/* Check if T2C compiled code exists for this block */
1176+
t2c_entry_t *t2c_entry =
1177+
cache_get(rv->t2c_cache, block->pc_start, false);
1178+
if (t2c_entry && t2c_entry->func) {
1179+
/* Clear compiled flag now that T2C code is available
1180+
* This allows the block to be evicted if needed */
1181+
atomic_store_explicit(&block->compiled, false,
1182+
memory_order_relaxed);
1183+
1184+
/* Execute the compiled function - no synchronization needed
1185+
* as T2C entries are immutable
1186+
*/
1187+
exec_t2c_func_t func = (exec_t2c_func_t) t2c_entry->func;
1188+
func(rv);
11261189
prev = NULL;
11271190
continue;
1128-
} /* check if invoking times of t1 generated code exceed threshold */
1129-
else if (!block->compiled && block->n_invoke >= THRESHOLD) {
1130-
block->compiled = true;
1191+
}
1192+
/* check if invoking times of t1 generated code exceed threshold */
1193+
if (!atomic_load_explicit(&block->compiled, memory_order_acquire) &&
1194+
block->n_invoke >= THRESHOLD) {
1195+
/* Mark block as queued for compilation to avoid re-queueing */
1196+
atomic_store_explicit(&block->compiled, true, memory_order_release);
1197+
11311198
queue_entry_t *entry = malloc(sizeof(queue_entry_t));
1132-
entry->block = block;
1133-
pthread_mutex_lock(&rv->wait_queue_lock);
1134-
list_add(&entry->list, &rv->wait_queue);
1135-
pthread_mutex_unlock(&rv->wait_queue_lock);
1199+
if (entry) {
1200+
/* Copy block metadata */
1201+
entry->pc_start = block->pc_start;
1202+
entry->pc_end = block->pc_end;
1203+
entry->n_insn = block->n_insn;
1204+
#if RV32_HAS(SYSTEM)
1205+
entry->satp = block->satp;
1206+
#endif
1207+
/* Deep copy the IR chain */
1208+
entry->ir_head_copy = NULL;
1209+
rv_insn_t **copy_ptr = &entry->ir_head_copy;
1210+
for (rv_insn_t *ir = block->ir_head; ir; ir = ir->next) {
1211+
rv_insn_t *ir_copy = malloc(sizeof(rv_insn_t));
1212+
if (!ir_copy) {
1213+
/* Clean up on failure */
1214+
for (rv_insn_t *tmp = entry->ir_head_copy, *next; tmp;
1215+
tmp = next) {
1216+
next = tmp->next;
1217+
free(tmp->fuse);
1218+
free(tmp);
1219+
}
1220+
free(entry);
1221+
atomic_store_explicit(&block->compiled, false,
1222+
memory_order_release);
1223+
goto skip_t2c;
1224+
}
1225+
memcpy(ir_copy, ir, sizeof(rv_insn_t));
1226+
/* Copy fuse data if present */
1227+
if (ir->fuse) {
1228+
size_t fuse_size = ir->imm2 * sizeof(opcode_fuse_t);
1229+
ir_copy->fuse = malloc(fuse_size);
1230+
if (ir_copy->fuse)
1231+
memcpy(ir_copy->fuse, ir->fuse, fuse_size);
1232+
}
1233+
/* Clear branch pointers as they are not needed for
1234+
* compilation.
1235+
*/
1236+
ir_copy->branch_taken = NULL;
1237+
ir_copy->branch_untaken = NULL;
1238+
ir_copy->branch_table = NULL;
1239+
/* Link the copy */
1240+
ir_copy->next = NULL;
1241+
*copy_ptr = ir_copy;
1242+
copy_ptr = &ir_copy->next;
1243+
}
1244+
1245+
pthread_mutex_lock(&rv->wait_queue_lock);
1246+
list_add(&entry->list, &rv->wait_queue);
1247+
pthread_cond_signal(&rv->wait_queue_cond);
1248+
pthread_mutex_unlock(&rv->wait_queue_lock);
1249+
/* Keep compiled flag set to prevent re-queueing */
1250+
} else {
1251+
/* Failed to allocate - clear compiled flag */
1252+
atomic_store_explicit(&block->compiled, false,
1253+
memory_order_release);
1254+
}
11361255
}
1256+
skip_t2c:; /* Empty statement for label */
11371257
#endif
11381258
/* executed through the tier-1 JIT compiler */
11391259
struct jit_state *state = rv->jit_state;
@@ -1165,6 +1285,12 @@ void rv_step(void *arg)
11651285
#endif
11661286
/* execute the block by interpreter */
11671287
const rv_insn_t *ir = block->ir_head;
1288+
/* should never happen */
1289+
if (unlikely(!ir || !ir->impl)) {
1290+
/* Fatal error: block is corrupted. Break to avoid infinite loop */
1291+
prev = NULL;
1292+
break;
1293+
}
11681294
if (unlikely(!ir->impl(rv, ir, rv->csr_cycle, rv->PC))) {
11691295
/* block should not be extended if execption handler invoked */
11701296
prev = NULL;

src/jit.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ typedef void (*exec_block_func_t)(riscv_t *rv, uintptr_t);
5858

5959
#if RV32_HAS(T2C)
6060
void t2c_compile(riscv_t *, block_t *);
61+
void t2c_compile_from_entry(riscv_t *, queue_entry_t *);
6162
typedef void (*exec_t2c_func_t)(riscv_t *);
6263

6364
/* The jit-cache records the program counters and the entries of executable

src/riscv.c

Lines changed: 37 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#include <assert.h>
77
#include <errno.h>
88
#include <fcntl.h>
9+
#include <stdatomic.h>
910
#include <stdio.h>
1011
#include <stdlib.h>
1112
#include <string.h>
@@ -206,19 +207,36 @@ static pthread_t t2c_thread;
206207
static void *t2c_runloop(void *arg)
207208
{
208209
riscv_t *rv = (riscv_t *) arg;
210+
pthread_mutex_lock(&rv->wait_queue_lock);
209211
while (!rv->quit) {
210-
if (!list_empty(&rv->wait_queue)) {
211-
queue_entry_t *entry =
212-
list_last_entry(&rv->wait_queue, queue_entry_t, list);
213-
pthread_mutex_lock(&rv->wait_queue_lock);
214-
list_del_init(&entry->list);
215-
pthread_mutex_unlock(&rv->wait_queue_lock);
216-
pthread_mutex_lock(&rv->cache_lock);
217-
t2c_compile(rv, entry->block);
218-
pthread_mutex_unlock(&rv->cache_lock);
219-
free(entry);
212+
/* Wait for work or quit signal */
213+
while (list_empty(&rv->wait_queue) && !rv->quit)
214+
pthread_cond_wait(&rv->wait_queue_cond, &rv->wait_queue_lock);
215+
216+
/* Check if we should quit */
217+
if (rv->quit)
218+
break;
219+
220+
/* Process the queue entry */
221+
queue_entry_t *entry =
222+
list_last_entry(&rv->wait_queue, queue_entry_t, list);
223+
list_del_init(&entry->list);
224+
pthread_mutex_unlock(&rv->wait_queue_lock);
225+
226+
/* Compile using the copied block data - thread safe */
227+
t2c_compile_from_entry(rv, entry);
228+
229+
/* Free the copied IR chain */
230+
for (rv_insn_t *ir = entry->ir_head_copy, *next; ir; ir = next) {
231+
next = ir->next;
232+
free(ir->fuse);
233+
free(ir);
220234
}
235+
free(entry);
236+
237+
pthread_mutex_lock(&rv->wait_queue_lock);
221238
}
239+
pthread_mutex_unlock(&rv->wait_queue_lock);
222240
return NULL;
223241
}
224242
#endif
@@ -732,7 +750,11 @@ riscv_t *rv_create(riscv_user_t rv_attr)
732750
/* prepare wait queue. */
733751
pthread_mutex_init(&rv->wait_queue_lock, NULL);
734752
pthread_mutex_init(&rv->cache_lock, NULL);
753+
pthread_cond_init(&rv->wait_queue_cond, NULL);
735754
INIT_LIST_HEAD(&rv->wait_queue);
755+
/* Create separate T2C cache for compiled blocks */
756+
rv->t2c_cache = cache_create(BLOCK_MAP_CAPACITY_BITS);
757+
assert(rv->t2c_cache);
736758
/* activate the background compilation thread. */
737759
pthread_create(&t2c_thread, NULL, t2c_runloop, rv);
738760
#endif
@@ -836,11 +858,16 @@ void rv_delete(riscv_t *rv)
836858
block_map_destroy(rv);
837859
#else
838860
#if RV32_HAS(T2C)
861+
pthread_mutex_lock(&rv->wait_queue_lock);
839862
rv->quit = true;
863+
pthread_cond_signal(&rv->wait_queue_cond);
864+
pthread_mutex_unlock(&rv->wait_queue_lock);
840865
pthread_join(t2c_thread, NULL);
841866
pthread_mutex_destroy(&rv->wait_queue_lock);
842867
pthread_mutex_destroy(&rv->cache_lock);
868+
pthread_cond_destroy(&rv->wait_queue_cond);
843869
jit_cache_exit(rv->jit_cache);
870+
cache_free(rv->t2c_cache);
844871
#endif
845872
jit_state_exit(rv->jit_state);
846873
cache_free(rv->block_cache);

0 commit comments

Comments
 (0)