Skip to content

memtrace/pt: make pid/tid frontend state per-core #19

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
iconstan01 wants to merge 2 commits into
base: main
Choose a base branch
from
+21 −19
Open

memtrace/pt: make pid/tid frontend state per-core #19

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9fef4c6
fix: make memtrace/pt frontend pid-tid state per-core and handle empt…
Feb 24, 2026
cbf50db
pt/memtrace: remove redundant memset and assert non-empty initial tra…
Mar 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 9 additions & 10 deletions src/frontend/pt_memtrace/memtrace_fe.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,10 @@ extern "C" {

static char* trace_files[MAX_NUM_PROCS];
TraceReader* trace_readers[MAX_NUM_PROCS];
// TODO: Make per proc?
uint64_t ins_id = 0;
uint64_t ins_id_fetched = 0;
uint64_t prior_tid = 0;
uint64_t prior_pid = 0;
uint64_t prior_tid[MAX_NUM_PROCS] = {0};
uint64_t prior_pid[MAX_NUM_PROCS] = {0};

extern scatter_info_map scatter_info_storage;

Expand Down Expand Up @@ -177,13 +176,13 @@ int memtrace_trace_read(int proc_id, ctype_pin_inst* next_onpath_pi) {
do {
insi = const_cast<InstInfo*>(trace_readers[proc_id]->nextInstruction());

if (prior_pid == 0) {
ASSERT(proc_id, prior_tid == 0);
if (prior_pid[proc_id] == 0) {
ASSERT(proc_id, prior_tid[proc_id] == 0);
ASSERT(proc_id, insi->valid);
prior_pid = insi->pid;
prior_tid = insi->tid;
ASSERT(proc_id, prior_tid);
ASSERT(proc_id, prior_pid);
prior_pid[proc_id] = insi->pid;
prior_tid[proc_id] = insi->tid;
ASSERT(proc_id, prior_tid[proc_id]);
ASSERT(proc_id, prior_pid[proc_id]);
}
if (insi->valid) {
ins_id++;
Expand All @@ -194,7 +193,7 @@ int memtrace_trace_read(int proc_id, ctype_pin_inst* next_onpath_pi) {
std::cout << "Reached end of trace" << std::endl;
return 0; // end of trace
}
} while (insi->pid != prior_pid || insi->tid != prior_tid);
} while (insi->pid != prior_pid[proc_id] || insi->tid != prior_tid[proc_id]);

if (insi->is_dr_ins) {
memcpy(next_onpath_pi, insi->info, sizeof(ctype_pin_inst));
Expand Down
16 changes: 9 additions & 7 deletions src/frontend/pt_memtrace/pt_fe.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@ extern "C" {
char* pt_trace_files[MAX_NUM_PROCS];
TraceReaderPT* pt_trace_readers[MAX_NUM_PROCS];
uint64_t pt_ins_id = 0;
uint64_t pt_prior_tid = 0;
uint64_t pt_prior_pid = 0;
uint64_t pt_prior_tid[MAX_NUM_PROCS] = {0};
uint64_t pt_prior_pid[MAX_NUM_PROCS] = {0};

std::mt19937 gen(0);
// Generate random addresses near the mean (1GB)
Expand Down Expand Up @@ -146,7 +146,7 @@ int pt_trace_read(int proc_id, ctype_pin_inst* pt_next_pi) {
pt_ins_id++;
if (!insi->valid)
return 0; // end of trace
} while (insi->pid != pt_prior_pid || insi->tid != pt_prior_tid);
} while (insi->pid != pt_prior_pid[proc_id] || insi->tid != pt_prior_tid[proc_id]);

init_ctype_pin_inst(pt_next_pi);
pt_fill_in_dynamic_info(pt_next_pi, insi);
Expand Down Expand Up @@ -179,6 +179,8 @@ void pt_init(void) {
uop_generator_init(NUM_CORES);
init_x86_decoder(nullptr);
init_x87_stack_delta();
memset(pt_prior_tid, 0, sizeof(pt_prior_tid));
memset(pt_prior_pid, 0, sizeof(pt_prior_pid));
Comment on lines +182 to +183
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These arrays are already value-initialized to zero at declaration (= {0}), so the additional memset is redundant. Consider removing these lines (or, if re-initialization at runtime is required, add a brief comment explaining why pt_init() needs to forcibly reset them).

Suggested change
memset(pt_prior_tid, 0, sizeof(pt_prior_tid));
memset(pt_prior_pid, 0, sizeof(pt_prior_pid));

Copilot uses AI. Check for mistakes.

// pt_next_pi = (ctype_pin_inst*)malloc(NUM_CORES * sizeof(ctype_pin_inst));
for (int i = 0; i < MAX_NUM_PROCS; ++i) {
Expand Down Expand Up @@ -236,8 +238,8 @@ void pt_setup(uns proc_id) {
std::cout << "Exit fast forward " << pt_ins_id << std::endl;
}

pt_prior_pid = insi->pid;
pt_prior_tid = insi->tid;
assert(pt_prior_tid);
assert(pt_prior_pid);
pt_prior_pid[proc_id] = insi->pid;
pt_prior_tid[proc_id] = insi->tid;
assert(pt_prior_tid[proc_id]);
assert(pt_prior_pid[proc_id]);
}
5 changes: 3 additions & 2 deletions src/frontend/pt_memtrace/trace_fe.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -370,8 +370,9 @@ void ext_trace_init() {
memtrace_init();

trace_buf_init();
for (uns proc_id = 0; proc_id < NUM_CORES; proc_id++)
trace_read(proc_id, &next_onpath_pi[proc_id]);
for (uns proc_id = 0; proc_id < NUM_CORES; proc_id++) {
ASSERT(proc_id, trace_read(proc_id, &next_onpath_pi[proc_id]));
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ASSERT will crash when the initial trace read legitimately returns 0 (end-of-trace / empty trace), which conflicts with the PR description stating empty initial trace reads should be handled safely. Instead of asserting, handle a 0 return explicitly (e.g., mark the core as having no trace / set an appropriate sentinel state / skip initialization for that core) and continue initialization without aborting.

Suggested change
ASSERT(proc_id, trace_read(proc_id, &next_onpath_pi[proc_id]));
auto trace_count = trace_read(proc_id, &next_onpath_pi[proc_id]);
if (trace_count == 0) {
// Legitimate end-of-trace or empty trace for this core; skip initialization.
DEBUG(proc_id, "No initial trace data for core %u", proc_id);
continue;
}
ASSERT(proc_id, trace_count > 0);

Copilot uses AI. Check for mistakes.
}
}

void ext_trace_done() {
Expand Down