From 181b259ea5e0d55ec42b4034e465aff066274f23 Mon Sep 17 00:00:00 2001 From: "chenwei.sun" Date: Tue, 27 May 2025 15:59:32 +0800 Subject: [PATCH 1/4] [SYCLomatic][CMake] Fix bug that implicit rules cannot be replaced by user-definded rules Signed-off-by: chenwei.sun --- .../DPCT/MigrateScript/MigrateCmakeScript.cpp | 19 +++++++++++++++-- ...le.cpp => case_044_test_dup_yaml_rule.cpp} | 0 .../dpct/cmake_migration/case_044/ref.txt | 4 ++-- .../case_068_fix_implicit_migration_rule.cpp | 9 ++++++++ .../cmake_migration/case_068/expected.txt | 19 +++++++++++++++++ .../dpct/cmake_migration/case_068/input.cmake | 21 +++++++++++++++++++ ...local_replace_implicit_migration_rule.yaml | 6 ++++++ .../case_068/migration_ref.log | 4 ++++ 8 files changed, 78 insertions(+), 4 deletions(-) rename clang/test/dpct/cmake_migration/case_044/{case_040_test_dup_yaml_rule.cpp => case_044_test_dup_yaml_rule.cpp} (100%) create mode 100644 clang/test/dpct/cmake_migration/case_068/case_068_fix_implicit_migration_rule.cpp create mode 100644 clang/test/dpct/cmake_migration/case_068/expected.txt create mode 100644 clang/test/dpct/cmake_migration/case_068/input.cmake create mode 100644 clang/test/dpct/cmake_migration/case_068/local_replace_implicit_migration_rule.yaml create mode 100644 clang/test/dpct/cmake_migration/case_068/migration_ref.log diff --git a/clang/lib/DPCT/MigrateScript/MigrateCmakeScript.cpp b/clang/lib/DPCT/MigrateScript/MigrateCmakeScript.cpp index ffb3b00dbc1d..63dc8d89931d 100644 --- a/clang/lib/DPCT/MigrateScript/MigrateCmakeScript.cpp +++ b/clang/lib/DPCT/MigrateScript/MigrateCmakeScript.cpp @@ -583,7 +583,22 @@ static void reserveImplicitMigrationRules() { for (const auto &Rule : ImplicitMigrationRules) { MetaRuleObject::PatternRewriter PrePR; PrePR.BuildScriptSyntax = Rule; - CmakeBuildInRules[PrePR.BuildScriptSyntax] = PrePR; + PrePR.RuleId = "implicit_rule_" + Rule; + + auto Iter = CmakeBuildInRules.find(PrePR.BuildScriptSyntax); + if (Iter != CmakeBuildInRules.end()) { + if (PrePR.Priority == RulePriority::Takeover && + Iter->second.Priority > PrePR.Priority) { + CmakeBuildInRules[PrePR.BuildScriptSyntax] = PrePR; + } else { + llvm::outs() << "[Warning]: Two migration rules (Rule:" << PrePR.RuleId + << ", Rule:" << Iter->second.RuleId + << ") are duplicated, the migration rule (Rule:" + << PrePR.RuleId << ") is ignored.\n"; + } + } else { + CmakeBuildInRules[PrePR.BuildScriptSyntax] = PrePR; + } } } @@ -610,7 +625,7 @@ void registerCmakeMigrationRule(MetaRuleObject &R) { Iter->second.Priority > PR.Priority) { CmakeBuildInRules[PR.BuildScriptSyntax] = PR; } else { - llvm::outs() << "[Warnning]: Two migration rules (Rule:" << R.RuleId + llvm::outs() << "[Warning]: Two migration rules (Rule:" << R.RuleId << ", Rule:" << Iter->second.RuleId << ") are duplicated, the migration rule (Rule:" << R.RuleId << ") is ignored.\n"; diff --git a/clang/test/dpct/cmake_migration/case_044/case_040_test_dup_yaml_rule.cpp b/clang/test/dpct/cmake_migration/case_044/case_044_test_dup_yaml_rule.cpp similarity index 100% rename from clang/test/dpct/cmake_migration/case_044/case_040_test_dup_yaml_rule.cpp rename to clang/test/dpct/cmake_migration/case_044/case_044_test_dup_yaml_rule.cpp diff --git a/clang/test/dpct/cmake_migration/case_044/ref.txt b/clang/test/dpct/cmake_migration/case_044/ref.txt index 91defe9ef010..da279889275c 100644 --- a/clang/test/dpct/cmake_migration/case_044/ref.txt +++ b/clang/test/dpct/cmake_migration/case_044/ref.txt @@ -1,2 +1,2 @@ -// CHECK: [Warnning]: Two migration rules (Rule:rule_gen_codes_add_link_options_test_dup, Rule:rule_gen_codes_add_link_options) are duplicated, the migration rule (Rule:rule_gen_codes_add_link_options_test_dup) is ignored. -// CHECK: [Warnning]: Two migration rules (Rule:rule_a_new, Rule:rule_a_old) are duplicated, the migration rule (Rule:rule_a_new) is ignored. +// CHECK: [Warning]: Two migration rules (Rule:rule_gen_codes_add_link_options_test_dup, Rule:rule_gen_codes_add_link_options) are duplicated, the migration rule (Rule:rule_gen_codes_add_link_options_test_dup) is ignored. +// CHECK: [Warning]: Two migration rules (Rule:rule_a_new, Rule:rule_a_old) are duplicated, the migration rule (Rule:rule_a_new) is ignored. diff --git a/clang/test/dpct/cmake_migration/case_068/case_068_fix_implicit_migration_rule.cpp b/clang/test/dpct/cmake_migration/case_068/case_068_fix_implicit_migration_rule.cpp new file mode 100644 index 000000000000..57f307aee658 --- /dev/null +++ b/clang/test/dpct/cmake_migration/case_068/case_068_fix_implicit_migration_rule.cpp @@ -0,0 +1,9 @@ +// RUN: rm -rf %T && mkdir -p %T +// RUN: cd %T +// RUN: cp %S/input.cmake ./input.cmake +// RUN: dpct -in-root ./ -out-root out ./input.cmake --migrate-build-script-only --rule-file=%S/local_replace_implicit_migration_rule.yaml > migration.log 2>&1 +// RUN: diff --strip-trailing-cr %S/expected.txt %T/out/input.cmake >> %T/diff.txt + +// RUN: cat %S/migration_ref.log > %T/check_migration_check.log +// RUN: cat %T/migration.log >>%T/check_migration_check.log +// RUN: FileCheck --match-full-lines --input-file %T/check_migration_check.log %T/check_migration_check.log diff --git a/clang/test/dpct/cmake_migration/case_068/expected.txt b/clang/test/dpct/cmake_migration/case_068/expected.txt new file mode 100644 index 000000000000..c005638305c8 --- /dev/null +++ b/clang/test/dpct/cmake_migration/case_068/expected.txt @@ -0,0 +1,19 @@ +## execute_process is commented in the CMakeLists.txt file +set(AOT_GENERATE_COMMAND + ${Python3_EXECUTABLE} -m aot_build_utils.generate --path + ${GENERATED_SOURCE_DIR} --head_dims ${HEAD_DIMS} --pos_encoding_modes + ${POS_ENCODING_MODES} --use_fp16_qk_reductions ${USE_FP16_QK_REDUCTIONS} + --mask_modes ${MASK_MODES} --enable_f16 ${FLASHINFER_ENABLE_F16} + --enable_bf16 ${FLASHINFER_ENABLE_BF16} --enable_fp8_e4m3 + ${FLASHINFER_ENABLE_FP8_E4M3} --enable_fp8_e5m2 + ${FLASHINFER_ENABLE_FP8_E5M2}) + +set(AOT_GENERATE_DISPATCH_INC_COMMAND + ${Python3_EXECUTABLE} -m aot_build_utils.generate_dispatch_inc --path + "${GENERATED_SOURCE_DIR}/dispatch.inc" --head_dims ${HEAD_DIMS} + --head_dims_sm90 ${HEAD_DIMS_SM90} --pos_encoding_modes + ${POS_ENCODING_MODES} --use_fp16_qk_reductions ${USE_FP16_QK_REDUCTIONS} + --mask_modes ${MASK_MODES}) + +#execute_process(COMMAND ${AOT_GENERATE_COMMAND} WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}) +#execute_process(COMMAND ${AOT_GENERATE_DISPATCH_INC_COMMAND} WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}) diff --git a/clang/test/dpct/cmake_migration/case_068/input.cmake b/clang/test/dpct/cmake_migration/case_068/input.cmake new file mode 100644 index 000000000000..6ad41d6c3796 --- /dev/null +++ b/clang/test/dpct/cmake_migration/case_068/input.cmake @@ -0,0 +1,21 @@ +## execute_process is commented in the CMakeLists.txt file +set(AOT_GENERATE_COMMAND + ${Python3_EXECUTABLE} -m aot_build_utils.generate --path + ${GENERATED_SOURCE_DIR} --head_dims ${HEAD_DIMS} --pos_encoding_modes + ${POS_ENCODING_MODES} --use_fp16_qk_reductions ${USE_FP16_QK_REDUCTIONS} + --mask_modes ${MASK_MODES} --enable_f16 ${FLASHINFER_ENABLE_F16} + --enable_bf16 ${FLASHINFER_ENABLE_BF16} --enable_fp8_e4m3 + ${FLASHINFER_ENABLE_FP8_E4M3} --enable_fp8_e5m2 + ${FLASHINFER_ENABLE_FP8_E5M2}) + +set(AOT_GENERATE_DISPATCH_INC_COMMAND + ${Python3_EXECUTABLE} -m aot_build_utils.generate_dispatch_inc --path + "${GENERATED_SOURCE_DIR}/dispatch.inc" --head_dims ${HEAD_DIMS} + --head_dims_sm90 ${HEAD_DIMS_SM90} --pos_encoding_modes + ${POS_ENCODING_MODES} --use_fp16_qk_reductions ${USE_FP16_QK_REDUCTIONS} + --mask_modes ${MASK_MODES}) + +execute_process(COMMAND ${AOT_GENERATE_COMMAND} + WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}) +execute_process(COMMAND ${AOT_GENERATE_DISPATCH_INC_COMMAND} + WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}) diff --git a/clang/test/dpct/cmake_migration/case_068/local_replace_implicit_migration_rule.yaml b/clang/test/dpct/cmake_migration/case_068/local_replace_implicit_migration_rule.yaml new file mode 100644 index 000000000000..a7c358b0b3eb --- /dev/null +++ b/clang/test/dpct/cmake_migration/case_068/local_replace_implicit_migration_rule.yaml @@ -0,0 +1,6 @@ +- Rule: rule_execute_process + Kind: CMakeRule + Priority: Takeover + CmakeSyntax: execute_process + In: execute_process(${args1} ${args2} ${args3}) + Out: "#execute_process(${args1} ${args2} ${args3})" diff --git a/clang/test/dpct/cmake_migration/case_068/migration_ref.log b/clang/test/dpct/cmake_migration/case_068/migration_ref.log new file mode 100644 index 000000000000..b0902f30bb4f --- /dev/null +++ b/clang/test/dpct/cmake_migration/case_068/migration_ref.log @@ -0,0 +1,4 @@ +// CHECK: Warning: Only CMake scripts will be migrated as no --migrate-build-script option is provided. +// CHECK: dpct exited with code: 4 (Migration of build script(s) completed.) +// CHECK: [Warning]: Two migration rules (Rule:implicit_rule_execute_process, Rule:rule_execute_process) are duplicated, the migration rule (Rule:implicit_rule_execute_process) is ignored. +// CHECK: Processing: {{(.+)}}/cmake_migration/case_068/Output/out/input.cmake From f257a9a06b87bac4381fd33a8b340063a2c1fbe7 Mon Sep 17 00:00:00 2001 From: "chenwei.sun" Date: Wed, 28 May 2025 08:21:24 +0800 Subject: [PATCH 2/4] fix CI lit test on Windows Signed-off-by: chenwei.sun --- clang/test/dpct/cmake_migration/case_068/migration_ref.log | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/test/dpct/cmake_migration/case_068/migration_ref.log b/clang/test/dpct/cmake_migration/case_068/migration_ref.log index b0902f30bb4f..f750ff8c22cc 100644 --- a/clang/test/dpct/cmake_migration/case_068/migration_ref.log +++ b/clang/test/dpct/cmake_migration/case_068/migration_ref.log @@ -1,4 +1,4 @@ // CHECK: Warning: Only CMake scripts will be migrated as no --migrate-build-script option is provided. // CHECK: dpct exited with code: 4 (Migration of build script(s) completed.) // CHECK: [Warning]: Two migration rules (Rule:implicit_rule_execute_process, Rule:rule_execute_process) are duplicated, the migration rule (Rule:implicit_rule_execute_process) is ignored. -// CHECK: Processing: {{(.+)}}/cmake_migration/case_068/Output/out/input.cmake +// CHECK: Processing: {{(.+)}}input.cmake From 66953dc83d5305290165c05c661e5d5fe234c922 Mon Sep 17 00:00:00 2001 From: "chenwei.sun" Date: Tue, 3 Jun 2025 15:03:00 +0800 Subject: [PATCH 3/4] address review comment Signed-off-by: chenwei.sun --- clang/lib/DPCT/MigrateScript/MigrateCmakeScript.cpp | 10 +++++----- .../dpct/cmake_migration/case_068/migration_ref.log | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/clang/lib/DPCT/MigrateScript/MigrateCmakeScript.cpp b/clang/lib/DPCT/MigrateScript/MigrateCmakeScript.cpp index 63dc8d89931d..21fcf9f63e7e 100644 --- a/clang/lib/DPCT/MigrateScript/MigrateCmakeScript.cpp +++ b/clang/lib/DPCT/MigrateScript/MigrateCmakeScript.cpp @@ -575,15 +575,15 @@ applyCmakeMigrationRules(const clang::tooling::UnifiedPath InRoot, // cmake systaxes need to be processed by implicit migration rules, as they are // difficult to be described with yaml based rule syntax. -static const std::vector ImplicitMigrationRules = { +static const std::vector BuiltinMigrationRules = { "cmake_minimum_required", "execute_process", "cuda_compile", "cuda_compile_fatbin"}; -static void reserveImplicitMigrationRules() { - for (const auto &Rule : ImplicitMigrationRules) { +static void reserveBuiltinMigrationRules() { + for (const auto &Rule : BuiltinMigrationRules) { MetaRuleObject::PatternRewriter PrePR; PrePR.BuildScriptSyntax = Rule; - PrePR.RuleId = "implicit_rule_" + Rule; + PrePR.RuleId = "builtin_rule_" + Rule; auto Iter = CmakeBuildInRules.find(PrePR.BuildScriptSyntax); if (Iter != CmakeBuildInRules.end()) { @@ -607,7 +607,7 @@ void doCmakeScriptMigration(const clang::tooling::UnifiedPath &InRoot, loadBufferFromFile(InRoot, OutRoot, CmakeScriptFileBufferMap); unifyInputFileFormat(CmakeScriptFileBufferMap, ScriptFileCRLFMap); convertAllCmakeCommandsToLowerCase(); - reserveImplicitMigrationRules(); + reserveBuiltinMigrationRules(); doCmakeScriptAnalysis(); applyCmakeMigrationRules(InRoot, OutRoot); storeBufferToFile(CmakeScriptFileBufferMap, ScriptFileCRLFMap); diff --git a/clang/test/dpct/cmake_migration/case_068/migration_ref.log b/clang/test/dpct/cmake_migration/case_068/migration_ref.log index f750ff8c22cc..dbcb502f9dff 100644 --- a/clang/test/dpct/cmake_migration/case_068/migration_ref.log +++ b/clang/test/dpct/cmake_migration/case_068/migration_ref.log @@ -1,4 +1,4 @@ // CHECK: Warning: Only CMake scripts will be migrated as no --migrate-build-script option is provided. // CHECK: dpct exited with code: 4 (Migration of build script(s) completed.) -// CHECK: [Warning]: Two migration rules (Rule:implicit_rule_execute_process, Rule:rule_execute_process) are duplicated, the migration rule (Rule:implicit_rule_execute_process) is ignored. +// CHECK: [Warning]: Two migration rules (Rule:builtin_rule_execute_process, Rule:rule_execute_process) are duplicated, the migration rule (Rule:builtin_rule_execute_process) is ignored. // CHECK: Processing: {{(.+)}}input.cmake From 26efd17939ef2e02c1ddfbc6258018d5758d95b3 Mon Sep 17 00:00:00 2001 From: "chenwei.sun" Date: Wed, 4 Jun 2025 09:20:58 +0800 Subject: [PATCH 4/4] further address review comments Signed-off-by: chenwei.sun --- .../DPCT/MigrateScript/MigrateCmakeScript.cpp | 31 ++++++++----------- 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/clang/lib/DPCT/MigrateScript/MigrateCmakeScript.cpp b/clang/lib/DPCT/MigrateScript/MigrateCmakeScript.cpp index 21fcf9f63e7e..aa2438c3bab4 100644 --- a/clang/lib/DPCT/MigrateScript/MigrateCmakeScript.cpp +++ b/clang/lib/DPCT/MigrateScript/MigrateCmakeScript.cpp @@ -579,25 +579,20 @@ static const std::vector BuiltinMigrationRules = { "cmake_minimum_required", "execute_process", "cuda_compile", "cuda_compile_fatbin"}; -static void reserveBuiltinMigrationRules() { +static void applyBuiltinMigrationRules() { for (const auto &Rule : BuiltinMigrationRules) { - MetaRuleObject::PatternRewriter PrePR; - PrePR.BuildScriptSyntax = Rule; - PrePR.RuleId = "builtin_rule_" + Rule; - - auto Iter = CmakeBuildInRules.find(PrePR.BuildScriptSyntax); - if (Iter != CmakeBuildInRules.end()) { - if (PrePR.Priority == RulePriority::Takeover && - Iter->second.Priority > PrePR.Priority) { - CmakeBuildInRules[PrePR.BuildScriptSyntax] = PrePR; - } else { - llvm::outs() << "[Warning]: Two migration rules (Rule:" << PrePR.RuleId - << ", Rule:" << Iter->second.RuleId - << ") are duplicated, the migration rule (Rule:" - << PrePR.RuleId << ") is ignored.\n"; - } + MetaRuleObject::PatternRewriter BuiltinPR; + BuiltinPR.BuildScriptSyntax = Rule; + BuiltinPR.RuleId = "builtin_rule_" + Rule; + + auto &PR = CmakeBuildInRules[Rule]; + if (PR.Priority != RulePriority::Takeover) { + PR = BuiltinPR; } else { - CmakeBuildInRules[PrePR.BuildScriptSyntax] = PrePR; + llvm::outs() << "[Warning]: Two migration rules (Rule:" + << BuiltinPR.RuleId << ", Rule:" << PR.RuleId + << ") are duplicated, the migration rule (Rule:" + << BuiltinPR.RuleId << ") is ignored.\n"; } } } @@ -607,7 +602,7 @@ void doCmakeScriptMigration(const clang::tooling::UnifiedPath &InRoot, loadBufferFromFile(InRoot, OutRoot, CmakeScriptFileBufferMap); unifyInputFileFormat(CmakeScriptFileBufferMap, ScriptFileCRLFMap); convertAllCmakeCommandsToLowerCase(); - reserveBuiltinMigrationRules(); + applyBuiltinMigrationRules(); doCmakeScriptAnalysis(); applyCmakeMigrationRules(InRoot, OutRoot); storeBufferToFile(CmakeScriptFileBufferMap, ScriptFileCRLFMap);