From 1517934105f238b5adfc0b29edd4ace9fe145de2 Mon Sep 17 00:00:00 2001 From: Derek Bruening Date: Thu, 21 Sep 2023 19:17:06 -0400 Subject: [PATCH 1/4] i#5694 core-sharded: Count threads in basic_counts Adds first-class counting of threads per shard in the basic_counts tool, for use with core-sharded operation. Updates the core-sharded tests with sanity checks. Issue: #5694 --- .../drcachesim/tests/core_sharded_test.cpp | 29 +++++++++++++++++++ clients/drcachesim/tools/basic_counts.cpp | 21 +++++++++----- clients/drcachesim/tools/basic_counts.h | 17 ++++++++--- 3 files changed, 55 insertions(+), 12 deletions(-) diff --git a/clients/drcachesim/tests/core_sharded_test.cpp b/clients/drcachesim/tests/core_sharded_test.cpp index 2345c3c9f18..6446f65f834 100644 --- a/clients/drcachesim/tests/core_sharded_test.cpp +++ b/clients/drcachesim/tests/core_sharded_test.cpp @@ -112,6 +112,9 @@ Total counts: 638938 total .* (.|\n)* Core [0-9] counts: +(.|\n)* + *[0-9]+ threads +(.|\n)* )DELIM"))); std::regex count("Core"); assert(std::distance(std::sregex_iterator(output.begin(), output.end(), count), @@ -128,6 +131,9 @@ Total counts: 638938 total .* (.|\n)* Core [0-9] counts: +(.|\n)* + *[0-9]+ threads +(.|\n)* )DELIM"))); std::regex count("Core"); assert(std::distance(std::sregex_iterator(output.begin(), output.end(), count), @@ -155,6 +161,9 @@ Total counts: 638938 total \(fetched\) instructions (.|\n)* Core [0-9] counts: +(.|\n)* + *[0-9]+ threads +(.|\n)* )DELIM"))); std::regex count("Core"); assert(std::distance(std::sregex_iterator(output.begin(), output.end(), count), @@ -178,27 +187,44 @@ Core [0-9] counts: assert(std::regex_search(output, std::regex(R"DELIM(Basic counts tool results: Total counts: 638938 total \(fetched\) instructions +(.|\n)* + 8 total threads (.|\n)* Core 5 counts: 175765 \(fetched\) instructions +(.|\n)* + 2 threads (.|\n)* Core 9 counts: 87891 \(fetched\) instructions +(.|\n)* + 1 threads (.|\n)* Core 0 counts: 87884 \(fetched\) instructions +(.|\n)* + 1 threads (.|\n)* Core 10? counts: 87875 \(fetched\) instructions +(.|\n)* + 1 threads (.|\n)* Core 10? counts: 87875 \(fetched\) instructions +(.|\n)* + 1 threads (.|\n)* Core 11 counts: 82508 \(fetched\) instructions +(.|\n)* + 1 threads (.|\n)* Core 8 counts: 29140 \(fetched\) instructions +(.|\n)* + 1 threads +(.|\n)* )DELIM"))); } { @@ -216,6 +242,9 @@ Total counts: 638938 total \(fetched\) instructions (.|\n)* Core .* +(.|\n)* + *[0-9]+ threads +(.|\n)* )DELIM"))); const char *replay_args[] = { "", "-core_sharded", "-simulator_type", "basic_counts", diff --git a/clients/drcachesim/tools/basic_counts.cpp b/clients/drcachesim/tools/basic_counts.cpp index de610bebfc0..a61b0bf89c8 100644 --- a/clients/drcachesim/tools/basic_counts.cpp +++ b/clients/drcachesim/tools/basic_counts.cpp @@ -128,6 +128,10 @@ basic_counts_t::parallel_shard_memref(void *shard_data, const memref_t &memref) { per_shard_t *per_shard = reinterpret_cast(shard_data); counters_t *counters = &per_shard->counters[per_shard->counters.size() - 1]; + if (memref.instr.tid != per_shard->last_tid_) { + counters->unique_threads.insert(memref.instr.tid); + per_shard->last_tid_ = memref.instr.tid; + } if (type_is_instr(memref.instr.type)) { ++counters->instrs; if (TESTANY(OFFLINE_FILE_TYPE_KERNEL_SYSCALLS, per_shard->filetype_)) { @@ -258,8 +262,8 @@ basic_counts_t::cmp_threads(const std::pair &l, } void -basic_counts_t::print_counters(const counters_t &counters, int64_t num_threads, - const std::string &prefix, bool for_kernel_trace) +basic_counts_t::print_counters(const counters_t &counters, const std::string &prefix, + bool for_kernel_trace) { std::cerr << std::setw(12) << counters.instrs << prefix << " (fetched) instructions\n"; @@ -282,8 +286,9 @@ basic_counts_t::print_counters(const counters_t &counters, int64_t num_threads, << " icache flushes\n"; std::cerr << std::setw(12) << counters.dcache_flushes << prefix << " dcache flushes\n"; - if (num_threads > 0) { - std::cerr << std::setw(12) << num_threads << prefix << " threads\n"; + if (shard_type_ != SHARD_BY_THREAD || counters.unique_threads.size() > 1) { + std::cerr << std::setw(12) << counters.unique_threads.size() << prefix + << " threads\n"; } std::cerr << std::setw(12) << counters.sched_markers << prefix << " scheduling markers\n"; @@ -331,7 +336,7 @@ basic_counts_t::print_results() total.shard_count = shard_map_.size(); std::cerr << TOOL_NAME << " results:\n"; std::cerr << "Total counts:\n"; - print_counters(total, shard_map_.size(), " total", for_kernel_trace); + print_counters(total, " total", for_kernel_trace); if (num_windows > 1) { std::cerr << "Total windows: " << num_windows << "\n"; @@ -339,7 +344,7 @@ basic_counts_t::print_results() std::cerr << "Window #" << i << ":\n"; for (const auto &shard : shard_map_) { if (shard.second->counters.size() > i) { - print_counters(shard.second->counters[i], 0, " window", + print_counters(shard.second->counters[i], " window", for_kernel_trace); } } @@ -355,7 +360,7 @@ basic_counts_t::print_results() std::cerr << "Thread " << keyvals.second->tid << " counts:\n"; else std::cerr << "Core " << keyvals.second->core << " counts:\n"; - print_counters(keyvals.second->counters[0], 0, "", for_kernel_trace); + print_counters(keyvals.second->counters[0], "", for_kernel_trace); } // TODO i#3599: also print thread-per-window stats. @@ -448,7 +453,7 @@ basic_counts_t::print_interval_results( << snapshot->interval_end_timestamp << ":\n"; counters_t diff = snapshot->counters; diff -= last; - print_counters(diff, 0, " interval delta"); + print_counters(diff, " interval delta"); last = snapshot->counters; if (knob_verbose_ > 0) { if (snapshot->instr_count_cumulative != diff --git a/clients/drcachesim/tools/basic_counts.h b/clients/drcachesim/tools/basic_counts.h index 51c7e9c6157..f9cf20b5605 100644 --- a/clients/drcachesim/tools/basic_counts.h +++ b/clients/drcachesim/tools/basic_counts.h @@ -119,6 +119,9 @@ class basic_counts_t : public analysis_tool_t { unique_pc_addrs.insert(addr); } } + for (const memref_tid_t tid : rhs.unique_threads) { + unique_threads.insert(tid); + } return *this; } counters_t & @@ -148,6 +151,9 @@ class basic_counts_t : public analysis_tool_t { for (const uint64_t addr : rhs.unique_pc_addrs) { unique_pc_addrs.erase(addr); } + for (const memref_tid_t tid : rhs.unique_threads) { + unique_threads.erase(tid); + } return *this; } bool @@ -172,7 +178,8 @@ class basic_counts_t : public analysis_tool_t { other_markers == rhs.other_markers && icache_flushes == rhs.icache_flushes && dcache_flushes == rhs.dcache_flushes && encodings == rhs.encodings && - unique_pc_addrs == rhs.unique_pc_addrs; + unique_pc_addrs == rhs.unique_pc_addrs && + unique_threads == rhs.unique_threads; } int64_t instrs = 0; int64_t instrs_nofetch = 0; @@ -198,6 +205,7 @@ class basic_counts_t : public analysis_tool_t { // we use encoding_is_new as a proxy. int64_t encodings = 0; std::unordered_set unique_pc_addrs; + std::unordered_set unique_threads; // Metadata for the counts. These are not used for the equality, increment, // or decrement operation, and must be set explicitly. @@ -239,6 +247,7 @@ class basic_counts_t : public analysis_tool_t { std::string error; intptr_t last_window = -1; intptr_t filetype_ = -1; + memref_tid_t last_tid_ = INVALID_THREAD_ID; /* Indicates whether we're currently in the kernel region of the trace, which * means we've seen a TRACE_MARKER_TYPE_SYSCALL_TRACE_START without its matching * TRACE_MARKER_TYPE_SYSCALL_TRACE_STOP. @@ -262,9 +271,9 @@ class basic_counts_t : public analysis_tool_t { static bool cmp_threads(const std::pair &l, const std::pair &r); - static void - print_counters(const counters_t &counters, int64_t num_threads, - const std::string &prefix, bool for_kernel_trace = false); + void + print_counters(const counters_t &counters, const std::string &prefix, + bool for_kernel_trace = false); void compute_shard_interval_result(per_shard_t *shard, uint64_t interval_id); From 782435f2d42cd859665e4fea08de4cf204adabda Mon Sep 17 00:00:00 2001 From: Derek Bruening Date: Thu, 21 Sep 2023 20:38:08 -0400 Subject: [PATCH 2/4] Always print thread count for total stats --- clients/drcachesim/tools/basic_counts.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/clients/drcachesim/tools/basic_counts.cpp b/clients/drcachesim/tools/basic_counts.cpp index a61b0bf89c8..9c66bf13366 100644 --- a/clients/drcachesim/tools/basic_counts.cpp +++ b/clients/drcachesim/tools/basic_counts.cpp @@ -286,7 +286,8 @@ basic_counts_t::print_counters(const counters_t &counters, const std::string &pr << " icache flushes\n"; std::cerr << std::setw(12) << counters.dcache_flushes << prefix << " dcache flushes\n"; - if (shard_type_ != SHARD_BY_THREAD || counters.unique_threads.size() > 1) { + if (shard_type_ != SHARD_BY_THREAD || counters.unique_threads.size() > 1 || + prefix == " total") { std::cerr << std::setw(12) << counters.unique_threads.size() << prefix << " threads\n"; } From 33df2c2c3084317f63c62fb6de2e7b6c386f28c6 Mon Sep 17 00:00:00 2001 From: Yang Liu Date: Sat, 23 Sep 2023 00:16:03 +0800 Subject: [PATCH 3/4] i#3544 RV64: Add handling of SYSCALL_METHOD_ECALL in mangle_syscall (#6321) Add missing handling of RISC-V system call method `SYSCALL_METHOD_ECALL` in `mangle_syscall()`. Issue: #3544 --- core/arch/mangle_shared.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/arch/mangle_shared.c b/core/arch/mangle_shared.c index e6bddefb7a0..4615edeedf6 100644 --- a/core/arch/mangle_shared.c +++ b/core/arch/mangle_shared.c @@ -663,7 +663,8 @@ mangle_syscall(dcontext_t *dcontext, instrlist_t *ilist, uint flags, instr_t *in if (get_syscall_method() != SYSCALL_METHOD_INT && get_syscall_method() != SYSCALL_METHOD_SYSCALL && get_syscall_method() != SYSCALL_METHOD_SYSENTER && - get_syscall_method() != SYSCALL_METHOD_SVC) { + get_syscall_method() != SYSCALL_METHOD_SVC && + get_syscall_method() != SYSCALL_METHOD_ECALL) { /* don't know convention on return address from kernel mode! */ SYSLOG_INTERNAL_ERROR("unsupported system call method"); LOG(THREAD, LOG_INTERP, 1, "don't know convention for this syscall method\n"); From 6ef42adb2a9754123855d113eb2262ef16f54fcb Mon Sep 17 00:00:00 2001 From: Derek Bruening Date: Fri, 22 Sep 2023 12:49:50 -0400 Subject: [PATCH 4/4] Review requests: range insert; named constant --- clients/drcachesim/tools/basic_counts.cpp | 5 +++-- clients/drcachesim/tools/basic_counts.h | 10 ++++------ 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/clients/drcachesim/tools/basic_counts.cpp b/clients/drcachesim/tools/basic_counts.cpp index 9c66bf13366..103f04bdda2 100644 --- a/clients/drcachesim/tools/basic_counts.cpp +++ b/clients/drcachesim/tools/basic_counts.cpp @@ -57,6 +57,7 @@ namespace dynamorio { namespace drmemtrace { const std::string basic_counts_t::TOOL_NAME = "Basic counts tool"; +const char *const basic_counts_t::TOTAL_COUNT_PREFIX = " total"; analysis_tool_t * basic_counts_tool_create(unsigned int verbose) @@ -287,7 +288,7 @@ basic_counts_t::print_counters(const counters_t &counters, const std::string &pr std::cerr << std::setw(12) << counters.dcache_flushes << prefix << " dcache flushes\n"; if (shard_type_ != SHARD_BY_THREAD || counters.unique_threads.size() > 1 || - prefix == " total") { + prefix == TOTAL_COUNT_PREFIX) { std::cerr << std::setw(12) << counters.unique_threads.size() << prefix << " threads\n"; } @@ -337,7 +338,7 @@ basic_counts_t::print_results() total.shard_count = shard_map_.size(); std::cerr << TOOL_NAME << " results:\n"; std::cerr << "Total counts:\n"; - print_counters(total, " total", for_kernel_trace); + print_counters(total, TOTAL_COUNT_PREFIX, for_kernel_trace); if (num_windows > 1) { std::cerr << "Total windows: " << num_windows << "\n"; diff --git a/clients/drcachesim/tools/basic_counts.h b/clients/drcachesim/tools/basic_counts.h index f9cf20b5605..a4247abf2ba 100644 --- a/clients/drcachesim/tools/basic_counts.h +++ b/clients/drcachesim/tools/basic_counts.h @@ -115,13 +115,10 @@ class basic_counts_t : public analysis_tool_t { dcache_flushes += rhs.dcache_flushes; encodings += rhs.encodings; if (track_unique_pc_addrs) { - for (const uint64_t addr : rhs.unique_pc_addrs) { - unique_pc_addrs.insert(addr); - } - } - for (const memref_tid_t tid : rhs.unique_threads) { - unique_threads.insert(tid); + unique_pc_addrs.insert(rhs.unique_pc_addrs.begin(), + rhs.unique_pc_addrs.end()); } + unique_threads.insert(rhs.unique_threads.begin(), rhs.unique_threads.end()); return *this; } counters_t & @@ -284,6 +281,7 @@ class basic_counts_t : public analysis_tool_t { std::mutex shard_map_mutex_; unsigned int knob_verbose_; static const std::string TOOL_NAME; + static const char *const TOTAL_COUNT_PREFIX; shard_type_t shard_type_ = SHARD_BY_THREAD; memtrace_stream_t *serial_stream_ = nullptr; };