ROCm / hipify_torch

MIT License
18 stars 10 forks source link

Use absolute paths for all file names to hipify. #44

Closed wenchenvincent closed 1 year ago

wenchenvincent commented 1 year ago

This fixes issues where some files with names in relative paths were not handled correctly.

pruthvistony commented 1 year ago

@wenchenvincent , The changes looks fine. But I havent come across this error. Can you please explain what was the issue happening due to relative paths for my understanding. Also I am not if any side effects of this change.

wenchenvincent commented 1 year ago

I had two issues, which were addressed by the two additions in the PR:

I had some relative paths in the header_include_dirs. So the all_files list would include the relative paths of these header files. But when hipify_torch was processing the include lines in a file, it would look up using the absolute file path. So those include will not be replaced with their hipified header files. I had to use the absolute path to work around this, but I felt it would be more robust to have hipify_torch do it.

I had some relative paths in the cuda_source_list. When replace_cuda_with_hip_files.py looks for the hipified list, hipified_result dict uses absolute paths as keys. So we would also need to supply the cuda_source_list with absolute paths.

jithunnair-amd commented 1 year ago

Requested @wenchenvincent to provide reproductions details offline for further debug. At first glance, it seems that the change in hipify_python.py should not be required, but the change in replace_cuda_with_hip_files.py looks correct.