Skip to content

Conversation

mwootton
Copy link
Contributor

Supersedes #1050

Convert to using rocprofiler-sdk instead of roctracer for collecting hip api calls and AMD gpu activity.

Reuses most existing roctracer infrastructure with a name for name replacement. Simultaneous support for both roctracer and rocprofiler-sdk was deemed impractical. This would require a whole new set of #ifdefs, a major refactor of the roctracer code, and additional build support. Even then, only one could be active at a time (and you wouldn't want both active).

In homage to the abandoned refactor, RocLogger.cpp/h were created to contain the rocprofbase classes and the api filter.

Roctracer has no established end date.
Rocprofiler-sdk is in rocm_3.1 forward.

This will create a dependency where (newest kineto on old rocm) and (old kineto on newest rocm) could fail to build with AMD gpu support. That window is already over 1 year wide.

@meta-cla meta-cla bot added the cla signed label Aug 25, 2025
@pruthvistony
Copy link
Contributor

pruthvistony commented Aug 26, 2025

@mwootton ,
Since this PR breaks internal builds based on older ROCm 6.2 release, can you update this PR and conditionalize it based on ROCm version.

ie.
use ROCTracer if ROCM_VERSION <6.4
and use ROCProf if ROCM_VERSION >=6.4

conditionally compile rocTracer file or rocProf files and keep both files in the repo.

cc @jeffdaily

@mwootton
Copy link
Contributor Author

mwootton commented Sep 9, 2025

Added support to conditionally compile the older roctracer implementation on rocm versions < 6.4.

@mwootton
Copy link
Contributor Author

@sraikund16 Could you take a look at this an confirm it functions properly on your internal builds?

@facebook-github-bot
Copy link
Contributor

@sraikund16 has imported this pull request. If you are a Meta employee, you can view this in D82773951.

@facebook-github-bot
Copy link
Contributor

@sraikund16 has imported this pull request. If you are a Meta employee, you can view this in D82773951.

@facebook-github-bot
Copy link
Contributor

@mwootton has updated the pull request. You must reimport the pull request before landing.

Copy link
Member

@aaronenyeshi aaronenyeshi left a comment

Choose a reason for hiding this comment

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

Review automatically exported from Phabricator review in Meta.

@facebook-github-bot
Copy link
Contributor

@mwootton has updated the pull request. You must reimport the pull request before landing.

Copy link

meta-codesync bot commented Oct 8, 2025

@sraikund16 has imported this pull request. If you are a Meta employee, you can view this in D82773951.

sraikund16 pushed a commit to sraikund16/kineto that referenced this pull request Oct 8, 2025
Summary:
Supersedes pytorch#1050

Convert to using rocprofiler-sdk instead of roctracer for collecting hip api calls and AMD gpu activity.

Reuses most existing roctracer infrastructure with a name for name replacement. Simultaneous support for both roctracer and rocprofiler-sdk was deemed impractical. This would require a whole new set of #ifdefs, a major refactor of the roctracer code, and additional build support. Even then, only one could be active at a time (and you wouldn't want both active).

In homage to the abandoned refactor, RocLogger.cpp/h were created to contain the rocprofbase classes and the api filter.

Roctracer has no established end date.
Rocprofiler-sdk is in rocm_3.1 forward.

This will create a dependency where (newest kineto on old rocm) and (old kineto on newest rocm) could fail to build with AMD gpu support. That window is already over 1 year wide.

Pull Request resolved: pytorch#1128

Reviewed By: malfet, aaronenyeshi

Differential Revision: D82773951

Pulled By: sraikund16
@facebook-github-bot
Copy link
Contributor

@mwootton has updated the pull request. You must reimport the pull request before landing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants