[Feature][PyTorch migration rule] Added PyTorch build script migration rule file #2564
Conversation
TejaX-Alaghari
force-pushed
the
py_mig
branch
2 times, most recently
from
274729a to
ab46091
Compare
TejaX-Alaghari
force-pushed
the
py_mig
branch
2 times, most recently
from
30509a6 to
628cd85
Compare
clang/lib/DPCT/ErrorHandle/Error.cpp
Outdated
| case MigratePythonBuildScriptSpecifiedButPythonRuleFileNotSpecified: | ||
| StatusString = | ||
| "Error: The option -migrate-build-script=Python requires python rule " | ||
| "file option --rule-file=python_migration_rule_[ipex|pytorch].yaml be " |
There was a problem hiding this comment.
| "file option --rule-file=python_migration_rule_[ipex|pytorch].yaml be " | |
| "rules to be explicitly loaded with option --rule-file, for example --rule-file={dpct_install_folder}/extensions/python_rules/python_build_script_migration_rule_[ipex|pytorch].yaml to load the predefined rules. |
There was a problem hiding this comment.
Done
Please verify
clang/lib/DPCT/ErrorHandle/Error.cpp
Outdated
| std::string getBuildScriptNotSpecifiedWarning() { | ||
| return "Warning: Only CMake scripts will be migrated as no " | ||
| "--migrate-build-script option is provided. " | ||
| "See https://www.intel.com/content/www/us/en/docs/" |
There was a problem hiding this comment.
suggest to remove the URL link. It's not necessary. And for SYCLomatic, it's better to provide URL to SYCLomatic doc
There was a problem hiding this comment.
I've removed the link now.
I will raise another PR for adding SYCLomatic doc link.
And in syclct, I'll later cherry-pick to remove it.
Is that fine?
| @@ -2389,7 +2389,7 @@ std::string DpctGlobalInfo::SYCLHeaderExtension = std::string(); | |||
| clang::tooling::UnifiedPath DpctGlobalInfo::CudaPath; | |||
| std::string DpctGlobalInfo::RuleFile = std::string(); | |||
| UsmLevel DpctGlobalInfo::UsmLvl = UsmLevel::UL_None; | |||
| BuildScriptKind DpctGlobalInfo::BuildScriptVal = BuildScriptKind::BS_None; | |||
| unsigned DpctGlobalInfo::BuildScriptType = 0; | |||
There was a problem hiding this comment.
if BuildScriptType default to zero, then it default to (BS_CMake), then migrateCMakeScripts() will be defatult return true. which conflict with option desc: "By default, no build script is migrated.\n" in DPCTOptions.inc.
There was a problem hiding this comment.
Here, BuildScriptType holds the value of the selected option by user (by cl::bits option), not the BS_CMake enum value.
So,
0 => No build script migration
1 => CMake script migration
2 => Python script migration
3 => Both CMake and Python script migration
clang/lib/DPCT/DPCT.cpp
Outdated
| if (MigrateBuildScriptOnly) { | ||
| if (!BuildScriptsSpecified) { | ||
| llvm::errs() << getBuildScriptNotSpecifiedWarning(); | ||
| DpctGlobalInfo::setBuildScript(1); // enable cmake script migration |
There was a problem hiding this comment.
| DpctGlobalInfo::setBuildScript(1); // enable cmake script migration | |
| DpctGlobalInfo::setBuildScript(1); // Please update magic number to meaning macro. |
There was a problem hiding this comment.
Added the logic to calculate bits value for CMake script selection (1 << BS_CMake)
|
The tests failure is due to expected ngt-check-options test resulting conflict between migrate-build-script-only and migrate-build-script This is because DAK_BuildScript being added to DPCT_OPTION_ACTIONS for --migrate-build-script (here) |
This PR introduces python_build_script_migration_rule_pytorch.yaml rule file for Python script migration using PyTorch's XPU class instead of IPEX's XPU