Skip to content
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.

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

Revert "i#5490 ind br tgt: Stop skip and switch at target marker (#6241)" #6248

Merged
merged 2 commits into from
Aug 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 3 additions & 5 deletions clients/drcachesim/reader/reader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -420,12 +420,10 @@ reader_t::skip_instructions_with_timestamp(uint64_t stop_instruction_count)
VPRINT(this, 4, "SKIP: type=%s (%d), size=%d, addr=0x%zx\n",
trace_type_names[next->type], next->type, next->size, next->addr);
// We need to pass up memrefs for the final skipped instr, but we don't
// want to process_input_entry() on the first unskipped instr (or, if it's an
// indirect branch: its target marker) so we can insert the timestamp+cpu first.
// want to process_input_entry() on the first unskipped instr so we can
// insert the timestamp+cpu first.
if (cur_instr_count_ + 1 == stop_count &&
(type_is_instr(static_cast<trace_type_t>(next->type)) ||
(next->type == TRACE_TYPE_MARKER &&
next->size == TRACE_MARKER_TYPE_BRANCH_TARGET))) {
type_is_instr(static_cast<trace_type_t>(next->type))) {
next_instr = *next;
break;
}
Expand Down
52 changes: 3 additions & 49 deletions clients/drcachesim/scheduler/scheduler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1367,14 +1367,6 @@ scheduler_tmpl_t<RecordType, ReaderType>::close_schedule_segment(output_ordinal_
// The end is exclusive, so use the max int value.
instr_ord = std::numeric_limits<uint64_t>::max();
}
if (input.switching_pre_instruction) {
input.switching_pre_instruction = false;
// We want to skip to the next instr on replay.
// reader_t::skip_instructions() will then back up one to this marker.
VPRINT(this, 3, "set_cur_input: +1 to instr_ord for indir branch for input=%d\n",
input.index);
++instr_ord;
}
VPRINT(this, 3,
"close_schedule_segment: input=%d start=%" PRId64 " stop=%" PRId64 "\n",
input.index, outputs_[output].record.back().start_instruction, instr_ord);
Expand Down Expand Up @@ -1679,34 +1671,6 @@ scheduler_tmpl_t<RecordType, ReaderType>::pick_next_input(output_ordinal_t outpu
return sched_type_t::STATUS_OK;
}

template <typename RecordType, typename ReaderType>
bool
scheduler_tmpl_t<RecordType, ReaderType>::at_pre_instruction_boundary(RecordType &record)
{
trace_marker_type_t marker_type;
uintptr_t marker_value;
if (record_type_is_marker(record, marker_type, marker_value) &&
marker_type == TRACE_MARKER_TYPE_BRANCH_TARGET) {
return true;
}
return false;
}

template <typename RecordType, typename ReaderType>
bool
scheduler_tmpl_t<RecordType, ReaderType>::at_instruction_boundary(RecordType &record,
bool &pre_instruction)
{
pre_instruction = false;
if (record_type_is_instr(record))
return true;
if (at_pre_instruction_boundary(record)) {
pre_instruction = true;
return true;
}
return false;
}

template <typename RecordType, typename ReaderType>
typename scheduler_tmpl_t<RecordType, ReaderType>::stream_status_t
scheduler_tmpl_t<RecordType, ReaderType>::next_record(output_ordinal_t output,
Expand Down Expand Up @@ -1795,46 +1759,37 @@ scheduler_tmpl_t<RecordType, ReaderType>::next_record(output_ordinal_t output,
// The stop is exclusive. 0 does mean to do nothing (easiest
// to have an empty record to share the next-entry for a start skip
// or other cases).
uint64_t ord = input->reader->get_instruction_ordinal();
if (ord >= stop ||
(ord + 1 >= stop && at_pre_instruction_boundary(record))) {
if (input->reader->get_instruction_ordinal() >= stop) {
need_new_input = true;
}
}
} else if (options_.mapping == MAP_TO_ANY_OUTPUT) {
trace_marker_type_t marker_type;
uintptr_t marker_value;
bool pre_instruction = false;
if (input->processing_blocking_syscall) {
// Wait until we're past all the markers associated with the syscall.
// XXX: We may prefer to stop before the return value marker for futex,
// or a kernel xfer marker, but our recorded format is on instr
// boundaries so we live with those being before the switch.
// We do stop before an indirect branch marker; this is also done
// in reader_t::skip_instructions(), so replay matches.
if (at_instruction_boundary(record, pre_instruction)) {
if (record_type_is_instr(record)) {
// Assume it will block and we should switch to a different input.
need_new_input = true;
in_wait_state = true;
input->processing_blocking_syscall = false;
if (pre_instruction)
input->switching_pre_instruction = true;
VPRINT(this, 3, "next_record[%d]: hit blocking syscall in input %d\n",
output, input->index);
}
} else if (record_type_is_marker(record, marker_type, marker_value) &&
marker_type == TRACE_MARKER_TYPE_MAYBE_BLOCKING_SYSCALL) {
input->processing_blocking_syscall = true;
} else if (options_.quantum_unit == QUANTUM_INSTRUCTIONS &&
at_instruction_boundary(record, pre_instruction)) {
record_type_is_instr(record)) {
++input->instrs_in_quantum;
if (input->instrs_in_quantum > options_.quantum_duration) {
// We again prefer to switch to another input even if the current
// input has the oldest timestamp, prioritizing context switches
// over timestamp ordering.
need_new_input = true;
if (pre_instruction)
input->switching_pre_instruction = true;
}
}
}
Expand Down Expand Up @@ -1898,7 +1853,6 @@ scheduler_tmpl_t<RecordType, ReaderType>::next_record(output_ordinal_t output,
}
break;
}
assert(!input->switching_pre_instruction); // Should only be locally set.
VPRINT(this, 4, "next_record[%d]: from %d: ", output, input->index);
VDO(this, 4, print_record(record););

Expand Down
11 changes: 0 additions & 11 deletions clients/drcachesim/scheduler/scheduler.h
Original file line number Diff line number Diff line change
Expand Up @@ -865,8 +865,6 @@ template <typename RecordType, typename ReaderType> class scheduler_tmpl_t {
uint64_t queue_counter = 0;
// Used to switch on the insruction *after* a blocking syscall.
bool processing_blocking_syscall = false;
// Used to switch on an indirect branch marker instead of the branch.
bool switching_pre_instruction = false;
};

// Format for recording a schedule to disk. A separate sequence of these records
Expand Down Expand Up @@ -989,15 +987,6 @@ template <typename RecordType, typename ReaderType> class scheduler_tmpl_t {
stream_status_t
next_record(output_ordinal_t output, RecordType &record, input_info_t *&input);

// If "record" is an instruction or an instruction-tied marker, returns true. For
// the marker case, sets pre_instruction to true; otherwise, to false.
bool
at_instruction_boundary(RecordType &record, bool &pre_instruction);

// Returns whether "record" is an instruction-tied marker.
bool
at_pre_instruction_boundary(RecordType &record);

// Skips ahead to the next region of interest if necessary.
// The caller must hold the input.lock.
stream_status_t
Expand Down
Binary file modified clients/drcachesim/tests/drmemtrace.allasm_x86_64.trace.zip
Binary file not shown.
6 changes: 3 additions & 3 deletions clients/drcachesim/tests/filter-asm.templatex
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,17 @@ Core #0 \(1 thread\(s\)\)
Miss rate: 100.00%
L1D0 .* stats:
Hits: 0
Misses: .*
Misses: 0
Compulsory misses: *[0-9,\.]*
Invalidations: 0.*
Invalidations: 0
Core #1 \(0 thread\(s\)\)
Core #2 \(0 thread\(s\)\)
Core #3 \(0 thread\(s\)\)
LL .* stats:
Hits: 0
Misses: .*
Compulsory misses: *[0-9,\.]*
Invalidations: 0.*
Invalidations: 0
Miss rate: 100.00%

===========================================================================
Expand Down
23 changes: 12 additions & 11 deletions clients/drcachesim/tests/offline-skip.expect
Original file line number Diff line number Diff line change
@@ -1,18 +1,19 @@
Output format:
<--record#-> <--instr#->: <---tid---> <record details>
------------------------------------------------------------
100 62: 1263623 <marker: timestamp 13335507464311957>
100 62: 1263623 <marker: tid 1263623 on core 7>
101 63: 1263623 ifetch 2 byte(s) @ 0x0000000000401026 0f 05 syscall -> %rcx %r11
102 63: 1263623 <marker: system call 1>
103 63: 1263623 <marker: maybe-blocking system call>
104 63: 1263623 <marker: timestamp 13335507464311968>
105 63: 1263623 <marker: tid 1263623 on core 7>
106 64: 1263623 ifetch 4 byte(s) @ 0x0000000000401028 48 83 eb 01 sub $0x0000000000000001 %rbx -> %rbx
107 65: 1263623 ifetch 4 byte(s) @ 0x000000000040102c 48 83 fb 00 cmp %rbx $0x0000000000000000
108 66: 1263623 ifetch 2 byte(s) @ 0x0000000000401030 75 d9 jnz $0x000000000040100b (taken)
86 62: 296231 <marker: timestamp 13319413770947383>
86 62: 296231 <marker: tid 296231 on core 10>
87 63: 296231 ifetch 2 byte(s) @ 0x0000000000401026 0f 05 syscall -> %rcx %r11
88 63: 296231 <marker: timestamp 13319413770947393>
89 63: 296231 <marker: tid 296231 on core 10>
90 64: 296231 ifetch 4 byte(s) @ 0x0000000000401028 48 83 eb 01 sub $0x0000000000000001 %rbx -> %rbx
91 65: 296231 ifetch 4 byte(s) @ 0x000000000040102c 48 83 fb 00 cmp %rbx $0x0000000000000000
92 66: 296231 ifetch 2 byte(s) @ 0x0000000000401030 75 d9 jnz $0x000000000040100b
93 67: 296231 ifetch 7 byte(s) @ 0x000000000040100b 48 c7 c7 01 00 00 00 mov $0x0000000000000001 -> %rdi
94 68: 296231 ifetch 8 byte(s) @ 0x0000000000401012 48 8d 34 25 00 20 40 lea 0x00402000 -> %rsi
94 68: 296231 00
View tool results:
4 : total instructions
6 : total instructions

===========================================================================
Trace invariant checks passed
22 changes: 11 additions & 11 deletions clients/drcachesim/tests/offline-skip2.expect
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
Output format:
<--record#-> <--instr#->: <---tid---> <record details>
------------------------------------------------------------
104 63: 1263623 <marker: timestamp 13335507464311968>
105 63: 1263623 <marker: tid 1263623 on core 7>
106 64: 1263623 ifetch 4 byte(s) @ 0x0000000000401028 48 83 eb 01 sub $0x0000000000000001 %rbx -> %rbx
107 65: 1263623 ifetch 4 byte(s) @ 0x000000000040102c 48 83 fb 00 cmp %rbx $0x0000000000000000
108 66: 1263623 ifetch 2 byte(s) @ 0x0000000000401030 75 d9 jnz $0x000000000040100b (taken)
109 67: 1263623 ifetch 7 byte(s) @ 0x000000000040100b 48 c7 c7 01 00 00 00 mov $0x0000000000000001 -> %rdi
110 68: 1263623 ifetch 8 byte(s) @ 0x0000000000401012 48 8d 34 25 00 20 40 lea 0x00402000 -> %rsi
110 68: 1263623 00
111 69: 1263623 ifetch 7 byte(s) @ 0x000000000040101a 48 c7 c2 0d 00 00 00 mov $0x000000000000000d -> %rdx
112 70: 1263623 ifetch 5 byte(s) @ 0x0000000000401021 b8 01 00 00 00 mov $0x00000001 -> %eax
113 71: 1263623 ifetch 2 byte(s) @ 0x0000000000401026 0f 05 syscall -> %rcx %r11
88 63: 296231 <marker: timestamp 13319413770947393>
89 63: 296231 <marker: tid 296231 on core 10>
90 64: 296231 ifetch 4 byte(s) @ 0x0000000000401028 48 83 eb 01 sub $0x0000000000000001 %rbx -> %rbx
91 65: 296231 ifetch 4 byte(s) @ 0x000000000040102c 48 83 fb 00 cmp %rbx $0x0000000000000000
92 66: 296231 ifetch 2 byte(s) @ 0x0000000000401030 75 d9 jnz $0x000000000040100b
93 67: 296231 ifetch 7 byte(s) @ 0x000000000040100b 48 c7 c7 01 00 00 00 mov $0x0000000000000001 -> %rdi
94 68: 296231 ifetch 8 byte(s) @ 0x0000000000401012 48 8d 34 25 00 20 40 lea 0x00402000 -> %rsi
94 68: 296231 00
95 69: 296231 ifetch 7 byte(s) @ 0x000000000040101a 48 c7 c2 0d 00 00 00 mov $0x000000000000000d -> %rdx
96 70: 296231 ifetch 5 byte(s) @ 0x0000000000401021 b8 01 00 00 00 mov $0x00000001 -> %eax
97 71: 296231 ifetch 2 byte(s) @ 0x0000000000401026 0f 05 syscall -> %rcx %r11
View tool results:
8 : total instructions

Expand Down
67 changes: 0 additions & 67 deletions clients/drcachesim/tests/raw2trace_unit_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2327,73 +2327,6 @@ test_branch_decoration(void *drcontext)
check_entry(entries, idx, TRACE_TYPE_THREAD_EXIT, -1) &&
check_entry(entries, idx, TRACE_TYPE_FOOTER, -1);
}
{
// Ensure indirect marker is not split by a chunk boundary.
static constexpr int CHUNK_SIZE = 3;
instrlist_t *ilist = instrlist_create(drcontext);
instr_t *nop0 = XINST_CREATE_nop(drcontext); // Avoid offset of 0.
instr_t *nop1 = XINST_CREATE_nop(drcontext);
instr_t *nop2 = XINST_CREATE_nop(drcontext);
instr_t *nop3 = XINST_CREATE_nop(drcontext);
instr_t *icall = XINST_CREATE_call_reg(drcontext, opnd_create_reg(REG1));
instr_t *nop4 = XINST_CREATE_nop(drcontext);
instrlist_append(ilist, nop0);
instrlist_append(ilist, nop1);
instrlist_append(ilist, nop2);
instrlist_append(ilist, nop3);
instrlist_append(ilist, icall);
instrlist_append(ilist, nop4);
size_t offs_nop0 = 0;
size_t offs_nop1 = offs_nop0 + instr_length(drcontext, nop0);
size_t offs_nop2 = offs_nop1 + instr_length(drcontext, nop1);
size_t offs_nop3 = offs_nop2 + instr_length(drcontext, nop2);
size_t offs_icall = offs_nop3 + instr_length(drcontext, nop3);
size_t offs_nop4 = offs_icall + instr_length(drcontext, icall);

std::vector<offline_entry_t> raw;
raw.push_back(make_header());
raw.push_back(make_tid());
raw.push_back(make_pid());
raw.push_back(make_line_size());
raw.push_back(make_block(offs_nop1, 4));
raw.push_back(make_block(offs_nop4, 1));
raw.push_back(make_exit());

std::vector<trace_entry_t> entries;
if (!run_raw2trace(drcontext, raw, ilist, entries, CHUNK_SIZE))
return false;
int idx = 0;
res = res && check_entry(entries, idx, TRACE_TYPE_HEADER, -1) &&
check_entry(entries, idx, TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_VERSION) &&
check_entry(entries, idx, TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_FILETYPE) &&
check_entry(entries, idx, TRACE_TYPE_THREAD, -1) &&
check_entry(entries, idx, TRACE_TYPE_PID, -1) &&
check_entry(entries, idx, TRACE_TYPE_MARKER,
TRACE_MARKER_TYPE_CACHE_LINE_SIZE) &&
check_entry(entries, idx, TRACE_TYPE_MARKER,
TRACE_MARKER_TYPE_CHUNK_INSTR_COUNT) &&
check_entry(entries, idx, TRACE_TYPE_ENCODING, -1) &&
check_entry(entries, idx, TRACE_TYPE_INSTR, -1, offs_nop1) &&
check_entry(entries, idx, TRACE_TYPE_ENCODING, -1) &&
check_entry(entries, idx, TRACE_TYPE_INSTR, -1, offs_nop2) &&
check_entry(entries, idx, TRACE_TYPE_ENCODING, -1) &&
check_entry(entries, idx, TRACE_TYPE_INSTR, -1, offs_nop3) &&
// Chunk split should be before branch target marker.
check_entry(entries, idx, TRACE_TYPE_MARKER,
TRACE_MARKER_TYPE_CHUNK_FOOTER) &&
check_entry(entries, idx, TRACE_TYPE_MARKER,
TRACE_MARKER_TYPE_RECORD_ORDINAL) &&
check_entry(entries, idx, TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_TIMESTAMP) &&
check_entry(entries, idx, TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_CPU_ID) &&
check_entry(entries, idx, TRACE_TYPE_ENCODING, -1) &&
check_entry(entries, idx, TRACE_TYPE_MARKER,
TRACE_MARKER_TYPE_BRANCH_TARGET) &&
check_entry(entries, idx, TRACE_TYPE_INSTR_INDIRECT_CALL, -1, offs_icall) &&
check_entry(entries, idx, TRACE_TYPE_ENCODING, -1) &&
check_entry(entries, idx, TRACE_TYPE_INSTR, -1, offs_nop4) &&
check_entry(entries, idx, TRACE_TYPE_THREAD_EXIT, -1) &&
check_entry(entries, idx, TRACE_TYPE_FOOTER, -1);
}
return res;
}

Expand Down
Loading