Skip to content

Commit a568fcf

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 f29a741 commit a568fcf

File tree

5 files changed

+258
-49
lines changed

5 files changed

+258
-49
lines changed

src/emulate.c

Lines changed: 146 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>
@@ -269,6 +271,7 @@ void rv_debug(riscv_t *rv)
269271
HASH_FUNC_IMPL(map_hash, BLOCK_MAP_CAPACITY_BITS, 1 << BLOCK_MAP_CAPACITY_BITS)
270272
#endif
271273

274+
272275
/* allocate a basic block */
273276
static block_t *block_alloc(riscv_t *rv)
274277
{
@@ -278,12 +281,13 @@ static block_t *block_alloc(riscv_t *rv)
278281
#if RV32_HAS(JIT)
279282
block->translatable = true;
280283
block->hot = false;
281-
block->hot2 = false;
282284
block->has_loops = false;
283285
block->n_invoke = 0;
284286
INIT_LIST_HEAD(&block->list);
285287
#if RV32_HAS(T2C)
286-
block->compiled = false;
288+
/* Initialize T2C compilation flag */
289+
atomic_store_explicit(&block->compiled, false, memory_order_relaxed);
290+
atomic_store_explicit(&block->func, NULL, memory_order_relaxed);
287291
#endif
288292
#endif
289293
return block;
@@ -918,6 +922,29 @@ static block_t *block_find_or_translate(riscv_t *rv)
918922
return next_blk;
919923
}
920924

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

@@ -930,43 +957,59 @@ static block_t *block_find_or_translate(riscv_t *rv)
930957
rv_insn_t *taken = entry->ir_tail->branch_taken,
931958
*untaken = entry->ir_tail->branch_untaken;
932959

933-
if (taken == replaced_blk_entry) {
960+
if (taken == replaced_blk_entry)
934961
entry->ir_tail->branch_taken = NULL;
935-
}
936-
if (untaken == replaced_blk_entry) {
962+
if (untaken == replaced_blk_entry)
937963
entry->ir_tail->branch_untaken = NULL;
938-
}
939964

940965
/* upadte JALR LUT */
941-
if (!entry->ir_tail->branch_table) {
966+
if (!entry->ir_tail->branch_table)
942967
continue;
943-
}
944968

945-
/**
946-
* TODO: upadate all JALR instructions which references to this
947-
* basic block as the destination.
969+
/* TODO: upadate all JALR instructions which references to this basic
970+
* block as the destination.
948971
*/
949972
}
950973

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;
974+
#if RV32_HAS(T2C)
975+
/* Double-check - do not evict if being compiled */
976+
if (atomic_load_explicit(&replaced_blk->compiled, memory_order_acquire)) {
977+
/* Block is being compiled - cannot evict */
978+
pthread_mutex_unlock(&rv->cache_lock);
955979

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

990+
return NULL;
991+
}
992+
#endif
993+
/* At this point, block is not compiled - safe to evict */
994+
/* Free the replaced block */
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,87 @@ 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+
exec_t2c_func_t func = (exec_t2c_func_t) t2c_entry->func;
1187+
func(rv);
11261188
prev = NULL;
11271189
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;
1190+
}
1191+
/* check if invoking times of t1 generated code exceed threshold */
1192+
if (!atomic_load_explicit(&block->compiled, memory_order_acquire) &&
1193+
block->n_invoke >= THRESHOLD) {
1194+
/* Mark block as queued for compilation to avoid re-queueing */
1195+
atomic_store_explicit(&block->compiled, true, memory_order_release);
1196+
11311197
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);
1198+
if (entry) {
1199+
/* Copy block metadata */
1200+
entry->pc_start = block->pc_start;
1201+
entry->pc_end = block->pc_end;
1202+
entry->n_insn = block->n_insn;
1203+
#if RV32_HAS(SYSTEM)
1204+
entry->satp = block->satp;
1205+
#endif
1206+
/* Deep copy the IR chain */
1207+
entry->ir_head_copy = NULL;
1208+
rv_insn_t **copy_ptr = &entry->ir_head_copy;
1209+
for (rv_insn_t *ir = block->ir_head; ir; ir = ir->next) {
1210+
rv_insn_t *ir_copy = malloc(sizeof(rv_insn_t));
1211+
if (!ir_copy) {
1212+
/* Clean up on failure */
1213+
for (rv_insn_t *tmp = entry->ir_head_copy, *next; tmp;
1214+
tmp = next) {
1215+
next = tmp->next;
1216+
free(tmp->fuse);
1217+
free(tmp);
1218+
}
1219+
free(entry);
1220+
atomic_store_explicit(&block->compiled, false,
1221+
memory_order_release);
1222+
goto skip_t2c;
1223+
}
1224+
memcpy(ir_copy, ir, sizeof(rv_insn_t));
1225+
/* Copy fuse data if present */
1226+
if (ir->fuse) {
1227+
size_t fuse_size = ir->imm2 * sizeof(opcode_fuse_t);
1228+
ir_copy->fuse = malloc(fuse_size);
1229+
if (ir_copy->fuse) {
1230+
memcpy(ir_copy->fuse, ir->fuse, fuse_size);
1231+
}
1232+
}
1233+
/* Clear branch pointers as they're not needed for
1234+
* compilation */
1235+
ir_copy->branch_taken = NULL;
1236+
ir_copy->branch_untaken = NULL;
1237+
ir_copy->branch_table = NULL;
1238+
/* Link the copy */
1239+
ir_copy->next = NULL;
1240+
*copy_ptr = ir_copy;
1241+
copy_ptr = &ir_copy->next;
1242+
}
1243+
1244+
pthread_mutex_lock(&rv->wait_queue_lock);
1245+
list_add(&entry->list, &rv->wait_queue);
1246+
pthread_cond_signal(&rv->wait_queue_cond);
1247+
pthread_mutex_unlock(&rv->wait_queue_lock);
1248+
/* Keep compiled flag set to prevent re-queueing */
1249+
} else {
1250+
/* Failed to allocate - clear compiled flag */
1251+
atomic_store_explicit(&block->compiled, false,
1252+
memory_order_release);
1253+
}
11361254
}
1255+
skip_t2c:; /* Empty statement for label */
11371256
#endif
11381257
/* executed through the tier-1 JIT compiler */
11391258
struct jit_state *state = rv->jit_state;

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: 39 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>
@@ -202,23 +203,42 @@ IO_HANDLER_IMPL(byte, write_b, W)
202203
#endif
203204

204205
#if RV32_HAS(T2C)
206+
/* Forward declarations removed - using reference counting instead */
207+
205208
static pthread_t t2c_thread;
206209
static void *t2c_runloop(void *arg)
207210
{
208211
riscv_t *rv = (riscv_t *) arg;
212+
pthread_mutex_lock(&rv->wait_queue_lock);
209213
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);
214+
/* Wait for work or quit signal */
215+
while (list_empty(&rv->wait_queue) && !rv->quit)
216+
pthread_cond_wait(&rv->wait_queue_cond, &rv->wait_queue_lock);
217+
218+
/* Check if we should quit */
219+
if (rv->quit)
220+
break;
221+
222+
/* Process the queue entry */
223+
queue_entry_t *entry =
224+
list_last_entry(&rv->wait_queue, queue_entry_t, list);
225+
list_del_init(&entry->list);
226+
pthread_mutex_unlock(&rv->wait_queue_lock);
227+
228+
/* Compile using the copied block data - thread safe */
229+
t2c_compile_from_entry(rv, entry);
230+
231+
/* Free the copied IR chain */
232+
for (rv_insn_t *ir = entry->ir_head_copy, *next; ir; ir = next) {
233+
next = ir->next;
234+
free(ir->fuse);
235+
free(ir);
220236
}
237+
free(entry);
238+
239+
pthread_mutex_lock(&rv->wait_queue_lock);
221240
}
241+
pthread_mutex_unlock(&rv->wait_queue_lock);
222242
return NULL;
223243
}
224244
#endif
@@ -732,7 +752,11 @@ riscv_t *rv_create(riscv_user_t rv_attr)
732752
/* prepare wait queue. */
733753
pthread_mutex_init(&rv->wait_queue_lock, NULL);
734754
pthread_mutex_init(&rv->cache_lock, NULL);
755+
pthread_cond_init(&rv->wait_queue_cond, NULL);
735756
INIT_LIST_HEAD(&rv->wait_queue);
757+
/* Create separate T2C cache for compiled blocks */
758+
rv->t2c_cache = cache_create(BLOCK_MAP_CAPACITY_BITS);
759+
assert(rv->t2c_cache);
736760
/* activate the background compilation thread. */
737761
pthread_create(&t2c_thread, NULL, t2c_runloop, rv);
738762
#endif
@@ -836,11 +860,16 @@ void rv_delete(riscv_t *rv)
836860
block_map_destroy(rv);
837861
#else
838862
#if RV32_HAS(T2C)
863+
pthread_mutex_lock(&rv->wait_queue_lock);
839864
rv->quit = true;
865+
pthread_cond_signal(&rv->wait_queue_cond);
866+
pthread_mutex_unlock(&rv->wait_queue_lock);
840867
pthread_join(t2c_thread, NULL);
841868
pthread_mutex_destroy(&rv->wait_queue_lock);
842869
pthread_mutex_destroy(&rv->cache_lock);
870+
pthread_cond_destroy(&rv->wait_queue_cond);
843871
jit_cache_exit(rv->jit_cache);
872+
cache_free(rv->t2c_cache);
844873
#endif
845874
jit_state_exit(rv->jit_state);
846875
cache_free(rv->block_cache);

0 commit comments

Comments
 (0)