-
Notifications
You must be signed in to change notification settings - Fork 58
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
Dromajo STF Trace Support Improvements #130
Comments
Sorry could you expand on what you mean by:
|
Sounds good to me, my comments: I like the generality of replacing no priv check. I also would like to know more about --stf-tracepoint It's pretty quick for me to run our process against a PR/draft PR when it's available. |
Tracepoints are just hint instructions with a special meaning.
In the current Dromajo STF tracing implementation, tracepoints are required to start and stop tracing. Adding this new option would make them optional. |
I was asking if new behavior was intended with --stf-tracepoint since the existing trace macros are enabled with --stf_trace and disabled without. |
Yes, so BTW I intended to follow the style of the existing parameters so |
@jeffnye-gh how would you like me to push my changes to the Condor Dromajo repo? I don't have write permissions so I can't push my branch. |
Does it work if you create a pull request from your fork of the Condor
Dromajo repo?
The method we use for riscv-perf-model, where we also do not have write
permission, was explained by knute
"... What folks do is fork the repo into their own private space, create
branches and push to those branches to their private forks. To push your
private branch to the main repo, you start by creating a new pull request
(I believe in your private fork) and set pull downs to the proper
source/destination locations."
…On Thu, Dec 21, 2023 at 3:19 PM Kathlene Magnus ***@***.***> wrote:
@jeffnye-gh <https://github.com/jeffnye-gh> how would you like me to push
my changes to the Condor Dromajo repo? I don't have write permissions so I
can't push my branch.
—
Reply to this email directly, view it on GitHub
<#130 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/A25MMC4IWVJ66SGHJWXV6Y3YKSRUXAVCNFSM6AAAAABAQKH6H6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNRWHE2DCNZYHA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
@jeffnye-gh I'll do that. Thanks! |
@kathlenehurt-sifive, when you're done with this issue, pass it over to me (I'll hijack it) and I'll move Olympia (via documentation) to illustrate the new flow. |
@klingaard this is good to go. Olympia should point to the master branch of Condor's Dromajo fork: https://github.com/Condor-Performance-Modeling/dromajo/tree/master The STF lib version also needs to be moved to the latest to pull in a required fix for the PC record. |
Thank you @kathlenemagnus! |
The SHA with KM's changes is commit ff13255 aka ff13255a50bd1b5e7597f3bf2ceaf24b782e8b72 |
I have been reviewing the STF trace generation support in Dromajo and have some recommendations to improve the implementation.
These are the current STF trace parameters in Dromajo:
I would like to propose removing
--stf_no_priv_check
and adding these parameters:These parameters would allow the user to specify whether tracepoints should be used to start/stop trace generation and specify which privilege modes to include in the trace. All conditions specified must be met for tracing to be enabled. These new options would also allow for noncontiguous traces, which is well handled by the STF library.
The text was updated successfully, but these errors were encountered: