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

add profiling timings for LLVM passes #50951

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

KristofferC
Copy link
Sponsor Member

A random screenshot of usage:

image

PIC->registerAfterPassCallback([isMetaPass](StringRef PassID, Any IR,
const PreservedAnalyses &PassPA) {
if (isMetaPass(PassID)) return;
if (jl_get_pgcstack() == NULL) return;
Copy link
Sponsor Member Author

Choose a reason for hiding this comment

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

Any opinions about this @topolarity

Copy link
Member

Choose a reason for hiding this comment

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

We don't need the jl_get_pgcstack() == NULL check any more, but it'd be really nice to implement #50926 (review) so that we're not just hiding LLVM work on its dedicated imaging threads

I'm going to take a whack at that later this week and see if we can land this with full thread visibility 👍

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

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

I think it is needed here because we later explicitly access the ptls.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah you're absolutely right

@brenhinkeller brenhinkeller added domain:tooling compiler:llvm For issues that relate to LLVM labels Aug 17, 2023
@KristofferC
Copy link
Sponsor Member Author

@topolarity any thoughts about this? Do you think this is good to go as perhaps an intermediate step? It has felt quite useful the times I have tried it out at least.

@gbaraldi
Copy link
Member

This looks pretty good IMO.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler:llvm For issues that relate to LLVM domain:tooling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants