Skip to content

Conversation

@zejun-chen
Copy link
Contributor

@zejun-chen zejun-chen commented Mar 26, 2025

This PR is used to change the interface for XPTI in kineto to be compatible with the PTI 0.11 release

@zejun-chen zejun-chen changed the title [XPTI] introduce new interface to kineto for PTI0.11 [WIP][XPTI] introduce new interface to kineto for PTI0.11 Mar 26, 2025
XPUPTI_CALL(ptiViewEnableRuntimeApi(1, pti_api_group_id::PTI_API_GROUP_SYCL, urUSMHostAlloc_id));
XPUPTI_CALL(ptiViewEnableRuntimeApi(1, pti_api_group_id::PTI_API_GROUP_SYCL, urUSMSharedAlloc_id));
XPUPTI_CALL(ptiViewEnableRuntimeApi(1, pti_api_group_id::PTI_API_GROUP_SYCL, urUSMDeviceAlloc_id));
}

Choose a reason for hiding this comment

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

why we still need these UR specific API ids? we should use API set?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. We will use API set, just apply the previous patch and open a draft PR.

Choose a reason for hiding this comment

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

then, can we make this PR as a draft?

@zejun-chen zejun-chen marked this pull request as draft March 27, 2025 00:59
@zejun-chen zejun-chen force-pushed the zejun/kineto_for_pti_0.11 branch from aebebe7 to a41d6f7 Compare April 1, 2025 01:17
@zejun-chen zejun-chen changed the title [WIP][XPTI] introduce new interface to kineto for PTI0.11 [WIP][XPTI] introduce new interface to kineto for PTI-0.11 Apr 1, 2025
@zejun-chen zejun-chen changed the title [WIP][XPTI] introduce new interface to kineto for PTI-0.11 [XPTI] introduce new interface to kineto for PTI-0.11 Apr 1, 2025
@zejun-chen zejun-chen marked this pull request as ready for review April 1, 2025 08:32
@zejun-chen
Copy link
Contributor Author

Hi, @aaronenyeshi

Could you help review this PR? It is used to update the xpu code in kineto to make it compatible for our new tracing tools.

Thank you.

@zejun-chen
Copy link
Contributor Author

Hi, @aaronenyeshi

Could you help review this PR? It is used to update the xpu code in kineto to make it compatible for our new tracing tools.

Thank you.

}
#endif

#if PTI_VERSION_MAJOR > 0 || PTI_VERSION_MINOR > 10
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be useful to use a version schema similar to cuda where they embed the MAJOR, MINOR and PATCH number into one number. If you use OR statements like this it can lead to more complexity if many ifdefs are needed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi, @sraikund16
Thank you for good suggestion.
For now, intel GPU doesn't have capability version control API. Only XPTI has release version macro so we use it here to do conditional building. We will consider to add the capability version control then refine here.
Thank you.

@facebook-github-bot
Copy link
Contributor

@sraikund16 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@sraikund16
Copy link
Contributor

Hi, @aaronenyeshi

Could you help review this PR? It is used to update the xpu code in kineto to make it compatible for our new tracing tools.

Thank you.

Aaron is not actively maintaining Kineto currently. You can reach out to me for future requests!

@zejun-chen
Copy link
Contributor Author

Hi, @aaronenyeshi
Could you help review this PR? It is used to update the xpu code in kineto to make it compatible for our new tracing tools.
Thank you.

Aaron is not actively maintaining Kineto currently. You can reach out to me for future requests!

Thank you @sraikund16 Really appreciate that
BTW, after you imported the PR, can we still push the commit to this PR? We may also need to do some modification, for example, use version schema for building.

@sraikund16
Copy link
Contributor

Hi, @aaronenyeshi
Could you help review this PR? It is used to update the xpu code in kineto to make it compatible for our new tracing tools.
Thank you.

Aaron is not actively maintaining Kineto currently. You can reach out to me for future requests!

Thank you @sraikund16 Really appreciate that BTW, after you imported the PR, can we still push the commit to this PR? We may also need to do some modification, for example, use version schema for building.

Absolutely, we can always re-import. Please update as you need.

@zejun-chen zejun-chen force-pushed the zejun/kineto_for_pti_0.11 branch from 7f811c2 to 378aab4 Compare April 21, 2025 00:43
@facebook-github-bot
Copy link
Contributor

@zejun-chen has updated the pull request. You must reimport the pull request before landing.

1 similar comment
@facebook-github-bot
Copy link
Contributor

@zejun-chen has updated the pull request. You must reimport the pull request before landing.

@zejun-chen zejun-chen changed the title [XPTI] introduce new interface to kineto for PTI-0.11 [XPTI] introduce new interface to kineto for PTI-0.11 and PTI-0.12 Apr 21, 2025
@zejun-chen
Copy link
Contributor Author

Hi, @sraikund16
Could you help review and reimport this PR? We use this PR to support the XPTI new release version(0.11 and 0.12 version).
Thank you.

@facebook-github-bot
Copy link
Contributor

@sraikund16 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@zejun-chen has updated the pull request. You must reimport the pull request before landing.

@facebook-github-bot
Copy link
Contributor

@sraikund16 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

1 similar comment
@facebook-github-bot
Copy link
Contributor

@sraikund16 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@sraikund16 merged this pull request in fb36cce.

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.

4 participants