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

i#4487: inline instr count for trace_after_instrs in drcachesim, AArch64 #4677

Merged
merged 12 commits into from
Feb 1, 2021

Conversation

sapostolakis
Copy link
Contributor

Inlines instruction counting for the -trace_after_instrs option in drcachesim on
AArch64 to avoid doing a clean call and thus improve performance.
Involves enabling drx_insert_counter_update() for 64-bit counters for AArch64.

Extends two tests with checks for 64-bit counter updates on AArch64.

Fixes: #4487

Inlines instruction counting for the -trace_after_instrs option in drcachesim on
AArch64 to avoid doing a clean call and thus improve performance.
It involves enabling drx_insert_counter_update() for 64-bit counters for AArch64.

Extends two tests with checks for 64-bit counter updates on AArch64.

Fixes: #4487
@derekbruening
Copy link
Contributor

@AssadHashmi -- If you could add @sapostolakis to the list for auto-triggering the aarch64 Jenkins. Though I thought any project member would trigger?

@derekbruening
Copy link
Contributor

run arm tests

clients/drcachesim/tracer/tracer.cpp Outdated Show resolved Hide resolved
clients/drcachesim/tracer/tracer.cpp Outdated Show resolved Hide resolved
clients/drcachesim/tracer/tracer.cpp Show resolved Hide resolved
clients/drcachesim/tracer/tracer.cpp Outdated Show resolved Hide resolved
ext/drx/drx.c Outdated Show resolved Hide resolved
ext/drx/drx.c Outdated Show resolved Hide resolved
@AssadHashmi
Copy link
Contributor

@AssadHashmi -- If you could add @sapostolakis to the list for auto-triggering the aarch64 Jenkins.

Added @sapostolakis to AArch64 CI Jenkins ACL.

Though I thought any project member would trigger?

Not yet. It's a manual process ATM.

clients/drcachesim/tracer/tracer.cpp Outdated Show resolved Hide resolved
ext/drx/drx.h Outdated Show resolved Hide resolved
ext/drx/drx.h Outdated Show resolved Hide resolved
ext/drx/drx.h Outdated Show resolved Hide resolved
ext/drx/drx.c Outdated Show resolved Hide resolved
api/docs/release.dox Outdated Show resolved Hide resolved
@sapostolakis sapostolakis merged commit a9be291 into master Feb 1, 2021
@sapostolakis sapostolakis deleted the i4487-AArchXX-64-bit-counters branch February 1, 2021 22:27
sapostolakis added a commit that referenced this pull request Feb 10, 2021
sapostolakis added a commit that referenced this pull request Feb 17, 2021
Revert certain x86-related changes from #4677. In particular, we avoid restoring the arithmetic flags and (if used) the scratch register before the call to `hit_instr_count_threshold`, which might not return. 
This fix is necessary but it seems to be causing failures in `drcachesim.delay-global` and `drcacheoff.max-global` on x86-64 CI testing. These failures are not reproducible outside CI testing and are (so far) inexplicable based on manual analysis of the instrumented assembly code (see #4711 (comment)). 
Thus, we are leaving x86_64 as technically broken to keep our tests green until the source of instability is found.

Issue: #4711
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[AArch64] Inline instr counting for trace_after_instrs in drcachesim
4 participants