Skip to content

[HIPIFY][SWDEV-490433][fix] Add support for cuda-samples helper headers to HIP #1962

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

Merged

Conversation

ranapratap55
Copy link
Contributor

@ranapratap55 ranapratap55 commented May 22, 2025

cuda-samples/* requires helper header files.
Fixes SWDEV-490433

@ranapratap55 ranapratap55 changed the title Add support for cuda-samples helper header to HIP [HIPIFY-CLANG] Add support for cuda-samples helper header to HIP May 22, 2025
@ranapratap55 ranapratap55 changed the title [HIPIFY-CLANG] Add support for cuda-samples helper header to HIP [HIPIFY][CLANG] Add support for cuda-samples helper header to HIP May 22, 2025
@emankov emankov changed the title [HIPIFY][CLANG] Add support for cuda-samples helper header to HIP [HIPIFY] Add support for cuda-samples helper headers to HIP May 22, 2025
@emankov emankov requested a review from kzhuravl May 22, 2025 13:06
@emankov emankov added HIP CUDA CUDA-related labels May 22, 2025
Copy link
Collaborator

@emankov emankov left a comment

Choose a reason for hiding this comment

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

Please regenerate hipify-perl and docs according to the instructions and add them to this PR. The needed tests I'll add in a separate PR.

@emankov
Copy link
Collaborator

emankov commented May 22, 2025

@ranapratap55,

Could you also provide here the statistics on CUDA Samples where helper includes are not provided explicitly in the sources? It is needed to figure out the necessity for implementing nvcc's behaviour with implicitly added includes in hipify-clang. We discussed it separately, and I need to be sure that it is quite a regular situation to implement on our side as well.

Thanks

@ranapratap55
Copy link
Contributor Author

@ranapratap55,

Could you also provide here the statistics on CUDA Samples where helper includes are not provided explicitly in the sources? It is needed to figure out the necessity for implementing nvcc's behaviour with implicitly added includes in hipify-clang. We discussed it separately, and I need to be sure that it is quite a regular situation to implement on our side as well.

Thanks

In CudaMath.h header file colorSums() uses float3 as input and it is implemented in helper_math.h.

The header CudaMath.h do not include helper_math.h in source. But it is included in the main source dxtc.cu file.

@emankov
Copy link
Collaborator

emankov commented May 22, 2025

In CudaMath.h header file colorSums() uses float3 as input and it is implemented in helper_math.h.

The header CudaMath.h do not include helper_math.h in source. But it is included in the main source dxtc.cu file.

Ok, this is an implicit nvcc include case, but only one. Do you have more evidence?

@ranapratap55
Copy link
Contributor Author

In CudaMath.h header file colorSums() uses float3 as input and it is implemented in helper_math.h.

The header CudaMath.h do not include helper_math.h in source. But it is included in the main source dxtc.cu file.

Ok, this is an implicit nvcc include case, but only one. Do you have more evidence?

There are multiple cuda-samples requires inclusion of those helper_* files. Below are few examples which I found while debugging requires including helper_* headers. I believe there are many more files which require those inclusions, and it is better we implement the generic solution as we might encounter these issues regularly.

  1. bodysystemcpu_impl.h
  2. MonteCarlo_reduction.cuh
  3. fastWalshTransform_kernel.cuh
  4. bicubicTexture_kernel.cuh

@ranapratap55 ranapratap55 force-pushed the ranapratap55/cuda-samples-helper-headers branch from e863e80 to a805bfb Compare May 29, 2025 06:25
@ranapratap55
Copy link
Contributor Author

Please regenerate hipify-perl and docs according to the instructions and add them to this PR. The needed tests I'll add in a separate PR.

Regenerated the hipify-perl and docs.

@emankov emankov added the fix It fixes bug label May 29, 2025
@emankov emankov changed the title [HIPIFY] Add support for cuda-samples helper headers to HIP [HIPIFY][SWDEV-490433][fix] Add support for cuda-samples helper headers to HIP May 29, 2025
Copy link
Collaborator

@emankov emankov left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@emankov emankov merged commit c530b59 into ROCm:amd-develop May 29, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CUDA CUDA-related fix It fixes bug HIP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants