[ROCm] Fixes and improvements to CUDA->HIP flag conversion for CPP extensions #149245
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/149245
Note: Links to docs will display an error until the docs builds have been completed. ✅ You can merge normally! (1 Unrelated Failure)As of commit f6e1b52 with merge base 790f93d ( UNSTABLE - The following job is marked as unstable, possibly due to flakiness on trunk:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
|
@Qubitium please sign the CLA so we can merge this PR. |
|
@jeffdaily @naromero77amd I was not able to count on ROCm/torch CI (broken) to run to check for CI regressions. Please make sure to compile full torch with the code and check/validate. Thanks. |
|
@naromero77amd @jeffdaily I think it would be good to update the PR notes and change To something more accurate for PR history. Because this message makes it sound like it's user's fault. And devs would say this is actually a missing piece of the hipify process and it should've been doing all along. Hipify, the process, overrides everything cuda, including compilers, so it makes total sense from developer's point of view that everything includes compiler flags. |
|
@Qubitium Updated description. As a co-author you should also be able to make edits to the title and/or description.
|
|
@pytorchbot rebase |
|
@pytorchbot started a rebase job onto refs/remotes/origin/viable/strict. Check the current status here |
Fixes ROCm/hip#3764. A user convenience.
I made improvements to the exising PR to that fixes CUDA to HIP flag conversion. * Log: We should log this behavior since this can break hipcc down the line for all we know * Log: Make it transparent to user what hipify actions are doing so devs are not left in the dark. * Fix cases where it should not touch the `-I` flags or case where CUDA appear more than once by replacing only the first instance. * Fix case where `nvcc` key may not exist * Fix case where hipify should ignore flag values and only touch the flag itself @naromero77amd cc @jeffdaily @sunway513 @jithunnair-amd @pruthvistony @ROCmSupport @dllehr-amd @jataylo @hongxiayang Passes the following unit test ```py import unittest import sys from unittest.mock import Mock class TestHipifyCompileFlags(unittest.TestCase): def test_hipify_compile_flags(self): class DummyClass: # Simple hipify, replace the first occurrence of CUDA with HIP # in flags starting with "-" and containing "CUDA", but exclude -I flags def _hipify_compile_flags(self, extension): if isinstance(extension.extra_compile_args, dict) and 'nvcc' in extension.extra_compile_args: modified_flags = [] for flag in extension.extra_compile_args['nvcc']: if flag.startswith("-") and "CUDA" in flag and not flag.startswith("-I"): # check/split flag into flag and value parts = flag.split("=", 1) if len(parts) == 2: flag_part, value_part = parts # replace fist instance of "CUDA" with "HIP" only in the flag and not flag value modified_flag_part = flag_part.replace("CUDA", "HIP", 1) modified_flag = f"{modified_flag_part}={value_part}" else: # replace fist instance of "CUDA" with "HIP" in flag modified_flag = flag.replace("CUDA", "HIP", 1) modified_flags.append(modified_flag) print(f'Modified flag: {flag} -> {modified_flag}', file=sys.stderr) else: modified_flags.append(flag) extension.extra_compile_args['nvcc'] = modified_flags dummy = DummyClass() extension = Mock() # (tested flag, expected flag) test_cases = [ ('-CUDA', '-HIP'), ('--CUDA', '--HIP'), ('--CUDA_CUDA', '--HIP_CUDA'), # only first CUDA instance ('-U__CUDA', '-U__HIP'), ('-D__CUDA', '-D__HIP'), ('-I/usr/local/CUDA', '-I/usr/local/CUDA'), # skip includes ('-Dmyflag=CUDA', '-Dmyflag=CUDA'), # should not be touched ] tested_flags = [test_case[0] for test_case in test_cases] expected_flags = [test_case[1] for test_case in test_cases] # test `nvcc` key exists extension.extra_compile_args = {'nvcc': tested_flags} dummy._hipify_compile_flags(extension) self.assertEqual(extension.extra_compile_args['nvcc'], expected_flags) # test `nvcc` key does not exists test_cases = {'abcd': ["-DTest", "--TEST"]} extension.extra_compile_args = test_cases dummy._hipify_compile_flags(extension) # Assert the result self.assertEqual(extension.extra_compile_args['abcd'], test_cases['abcd']) if __name__ == '__main__': unittest.main() ```
|
Successfully rebased |
1713839 to
f6e1b52
Compare
|
@pytorchbot merge |
Merge startedYour change will be merged once all checks pass (ETA 0-4 Hours). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
|
@pytorchbot cherry-pick --onto release/2.7 -c critical |
…tensions (#149245) Fixes ROCm/hip#3764. Fixes and improvements to CUDA->HIP flag conversion for CPP extensions - Log flag conversion for debugging purposes. - Fix cases where it should not touch the -I flags or cases where CUDA appears more than once by replacing only the first instance. - Fix case where nvcc key may not exist - Fix case where hipify should ignore flag values and only touch the flag itself Pull Request resolved: #149245 Approved by: https://github.com/jeffdaily Co-authored-by: Qubitium-ModelCloud <qubitium@modelcloud.ai> (cherry picked from commit c0566e0)
Cherry picking #149245The cherry pick PR is at #149432 and it is recommended to link a critical cherry pick PR with an issue. The following tracker issues are updated: Details for Dev Infra teamRaised by workflow job |
…tensions (#149432) [ROCm] Fixes and improvements to CUDA->HIP flag conversion for CPP extensions (#149245) Fixes ROCm/hip#3764. Fixes and improvements to CUDA->HIP flag conversion for CPP extensions - Log flag conversion for debugging purposes. - Fix cases where it should not touch the -I flags or cases where CUDA appears more than once by replacing only the first instance. - Fix case where nvcc key may not exist - Fix case where hipify should ignore flag values and only touch the flag itself Pull Request resolved: #149245 Approved by: https://github.com/jeffdaily Co-authored-by: Qubitium-ModelCloud <qubitium@modelcloud.ai> (cherry picked from commit c0566e0) Co-authored-by: Nichols A. Romero <nick.romero@amd.com>
Fixes ROCm/hip#3764.
Fixes and improvements to CUDA->HIP flag conversion for CPP extensions
cc @jeffdaily @sunway513 @jithunnair-amd @pruthvistony @ROCmSupport @dllehr-amd @jataylo @hongxiayang