From 08518c607ba81d9eea91f57fd2aae537aa3cca36 Mon Sep 17 00:00:00 2001 From: "Larsen, Steffen" Date: Thu, 9 Mar 2023 00:51:16 -0800 Subject: [PATCH 1/2] [SYCL] Implement two-run aspect propagation This commit splits aspect propagation into two runs: 1. First run propagates all aspects, except fp64. Warnings are still issued for fp64 as if it was fully propagated, but the resulting metadata will not reflect it. This run before optimizations. 2. Second run propagates all aspects, including fp64. This should not have any effect on already propagated aspects. This run will not issue warnings as any conflicts would have been reported by the first pass. Signed-off-by: Larsen, Steffen --- clang/lib/CodeGen/BackendUtil.cpp | 5 +- .../SYCLLowerIR/SYCLPropagateAspectsUsage.h | 8 +- .../SYCLLowerIR/SYCLPropagateAspectsUsage.cpp | 61 +++++++- .../double-prop-after-exclude.ll | 48 ++++++ .../PropagateAspectsUsage/exclude-aspect.ll | 139 ++++++++++++++++++ 5 files changed, 252 insertions(+), 9 deletions(-) create mode 100644 llvm/test/SYCLLowerIR/PropagateAspectsUsage/double-prop-after-exclude.ll create mode 100644 llvm/test/SYCLLowerIR/PropagateAspectsUsage/exclude-aspect.ll diff --git a/clang/lib/CodeGen/BackendUtil.cpp b/clang/lib/CodeGen/BackendUtil.cpp index d7b1dcf72e8ca..95a89a0a6f64d 100644 --- a/clang/lib/CodeGen/BackendUtil.cpp +++ b/clang/lib/CodeGen/BackendUtil.cpp @@ -911,7 +911,7 @@ void EmitAssemblyHelper::RunOptimizationPipeline( PB.registerPipelineStartEPCallback( [&](ModulePassManager &MPM, OptimizationLevel Level) { MPM.addPass(ESIMDVerifierPass(LangOpts.SYCLESIMDForceStatelessMem)); - MPM.addPass(SYCLPropagateAspectsUsagePass()); + MPM.addPass(SYCLPropagateAspectsUsagePass({"fp64"})); }); // Add the InferAddressSpaces pass for all the SPIR[V] targets @@ -1026,6 +1026,9 @@ void EmitAssemblyHelper::RunOptimizationPipeline( if (LangOpts.EnableDAEInSpirKernels) MPM.addPass(DeadArgumentEliminationSYCLPass()); + // Rerun aspect propagation without warning diagnostics. + MPM.addPass(SYCLPropagateAspectsUsagePass({}, /*ValidateAspects=*/false)); + // Add SPIRITTAnnotations pass to the pass manager if // -fsycl-instrument-device-code option was passed. This option can be // used only with spir triple. diff --git a/llvm/include/llvm/SYCLLowerIR/SYCLPropagateAspectsUsage.h b/llvm/include/llvm/SYCLLowerIR/SYCLPropagateAspectsUsage.h index 4845ad6e97bf1..f8f1078b0669a 100644 --- a/llvm/include/llvm/SYCLLowerIR/SYCLPropagateAspectsUsage.h +++ b/llvm/include/llvm/SYCLLowerIR/SYCLPropagateAspectsUsage.h @@ -20,13 +20,19 @@ namespace llvm { class SYCLPropagateAspectsUsagePass : public PassInfoMixin { public: - SYCLPropagateAspectsUsagePass(StringRef OptionsString = {}) { + SYCLPropagateAspectsUsagePass(std::set ExcludeAspects = {}, + bool ValidateAspects = true, + StringRef OptionsString = {}) + : ExcludedAspects{std::move(ExcludeAspects)}, + ValidateAspectUsage{ValidateAspects} { OptionsString.split(this->TargetFixedAspects, ',', /*MaxSplit=*/-1, /*KeepEmpty=*/false); }; PreservedAnalyses run(Module &M, ModuleAnalysisManager &); private: + std::set ExcludedAspects; + const bool ValidateAspectUsage; SmallVector TargetFixedAspects; }; diff --git a/llvm/lib/SYCLLowerIR/SYCLPropagateAspectsUsage.cpp b/llvm/lib/SYCLLowerIR/SYCLPropagateAspectsUsage.cpp index b13e480170fec..0f32fd78b68de 100644 --- a/llvm/lib/SYCLLowerIR/SYCLPropagateAspectsUsage.cpp +++ b/llvm/lib/SYCLLowerIR/SYCLPropagateAspectsUsage.cpp @@ -53,6 +53,11 @@ static cl::opt ClSyclFixedTargets( "is expected to be runnable on"), cl::Hidden, cl::init("")); +static cl::opt ClSyclExcludeAspects( + "sycl-propagate-aspects-usage-exclude-aspects", + cl::desc("Specify aspects to exclude when propagating aspect usage"), + cl::Hidden, cl::init("")); + namespace { using AspectsSetTy = SmallSet; @@ -293,15 +298,37 @@ getAspectUsageChain(const Function *F, const FunctionToAspectsMapTy &AspectsMap, return CallChain; } -void createUsedAspectsMetadataForFunctions(FunctionToAspectsMapTy &Map) { +void createUsedAspectsMetadataForFunctions( + FunctionToAspectsMapTy &Map, const AspectsSetTy &ExcludeAspectVals) { for (auto &[F, Aspects] : Map) { if (Aspects.empty()) continue; LLVMContext &C = F->getContext(); + // Create a set of unique aspects. First we add the ones from the found + // aspects that have not been excluded. + AspectsSetTy UniqueAspects; + for (const int &A : Aspects) + if (!ExcludeAspectVals.contains(A)) + UniqueAspects.insert(A); + + // If there are no new aspects, we can just keep the old metadata. + if (UniqueAspects.empty()) + continue; + + // If there is new metadata, merge it with the old aspects. We preserve + // the excluded ones. + if (const MDNode *ExistingAspects = F->getMetadata("sycl_used_aspects")) { + for (const MDOperand &MDOp : ExistingAspects->operands()) { + const Constant *C = cast(MDOp)->getValue(); + UniqueAspects.insert(cast(C)->getSExtValue()); + } + } + + // Create new metadata. SmallVector AspectsMetadata; - for (const auto &A : Aspects) + for (const int &A : UniqueAspects) AspectsMetadata.push_back(ConstantAsMetadata::get( ConstantInt::getSigned(Type::getInt32Ty(C), A))); @@ -506,7 +533,8 @@ void setSyclFixedTargetsMD(const std::vector &EntryPoints, FunctionToAspectsMapTy buildFunctionsToAspectsMap(Module &M, TypeToAspectsMapTy &TypesWithAspects, const AspectValueToNameMapTy &AspectValues, - const std::vector &EntryPoints) { + const std::vector &EntryPoints, + bool ValidateAspects) { FunctionToAspectsMapTy FunctionToUsedAspects; FunctionToAspectsMapTy FunctionToDeclaredAspects; CallGraphTy CG; @@ -522,8 +550,9 @@ buildFunctionsToAspectsMap(Module &M, TypeToAspectsMapTy &TypesWithAspects, for (Function *F : EntryPoints) propagateAspectsThroughCG(F, CG, FunctionToUsedAspects, Visited); - validateUsedAspectsForFunctions(FunctionToUsedAspects, AspectValues, - EntryPoints, CG); + if (ValidateAspects) + validateUsedAspectsForFunctions(FunctionToUsedAspects, AspectValues, + EntryPoints, CG); // The set of aspects from FunctionToDeclaredAspects should be merged to the // set of FunctionToUsedAspects after validateUsedAspectsForFunctions call to @@ -558,6 +587,14 @@ SYCLPropagateAspectsUsagePass::run(Module &M, ModuleAnalysisManager &MAM) { StringRef(ClSyclFixedTargets) .split(TargetFixedAspects, ',', /*MaxSplit=*/-1, /*KeepEmpty=*/false); + if (ClSyclExcludeAspects.getNumOccurrences() > 0) { + SmallVector ExcludedAspectsVec; + StringRef(ClSyclExcludeAspects) + .split(ExcludedAspectsVec, ',', /*MaxSplit=*/-1, /*KeepEmpty=*/false); + ExcludedAspects.insert(ExcludedAspectsVec.begin(), + ExcludedAspectsVec.end()); + } + std::vector EntryPoints; for (Function &F : M.functions()) if (isEntryPoint(F)) @@ -566,9 +603,19 @@ SYCLPropagateAspectsUsagePass::run(Module &M, ModuleAnalysisManager &MAM) { propagateAspectsToOtherTypesInModule(M, TypesWithAspects, AspectValues); FunctionToAspectsMapTy FunctionToUsedAspects = buildFunctionsToAspectsMap( - M, TypesWithAspects, AspectValues, EntryPoints); + M, TypesWithAspects, AspectValues, EntryPoints, ValidateAspectUsage); + + // Create a set of excluded aspect values. + AspectsSetTy ExcludedAspectVals; + for (const StringRef &AspectName : ExcludedAspects) { + const auto AspectValIter = AspectValues.find(AspectName); + assert(AspectValIter != AspectValues.end() && + "Excluded aspect does not have a corresponding value."); + ExcludedAspectVals.insert(AspectValIter->second); + } - createUsedAspectsMetadataForFunctions(FunctionToUsedAspects); + createUsedAspectsMetadataForFunctions(FunctionToUsedAspects, + ExcludedAspectVals); setSyclFixedTargetsMD(EntryPoints, TargetFixedAspects, AspectValues); diff --git a/llvm/test/SYCLLowerIR/PropagateAspectsUsage/double-prop-after-exclude.ll b/llvm/test/SYCLLowerIR/PropagateAspectsUsage/double-prop-after-exclude.ll new file mode 100644 index 0000000000000..8eb2512b507d6 --- /dev/null +++ b/llvm/test/SYCLLowerIR/PropagateAspectsUsage/double-prop-after-exclude.ll @@ -0,0 +1,48 @@ +; RUN: opt -passes=sycl-propagate-aspects-usage -sycl-propagate-aspects-usage-exclude-aspects=fp64 < %s -S -o %t_first.ll +; RUN: opt -passes=sycl-propagate-aspects-usage < %t_first.ll -S -o %t_second.ll +; FileCheck %s --input-file %t_first.ll --check-prefix=CHECK-FIRST +; FileCheck %s --input-file %t_second.ll --check-prefix=CHECK-SECOND +; +; Test checks that fp64 usage is correctly propagate in the two-run model. + +%composite = type { double } + +; CHECK-FIRST-NOT: spir_kernel void @kernel() {{.*}} !sycl_used_aspects +; CHECK-SECOND: spir_kernel void @kernel() !sycl_used_aspects ![[MDID:]] +define spir_kernel void @kernel() { + call spir_func void @func() + ret void +} + +; CHECK-FIRST-NOT: spir_func void @func() {{.*}} !sycl_used_aspects +; CHECK-SECOND: spir_func void @func() !sycl_used_aspects ![[MDID]] { +define spir_func void @func() { + %tmp = alloca double + ret void +} + +; CHECK-FIRST-NOT: spir_func void @func.array() {{.*}} !sycl_used_aspects +; CHECK-SECOND: spir_func void @func.array() !sycl_used_aspects ![[MDID]] { +define spir_func void @func.array() { + %tmp = alloca [4 x double] + ret void +} + +; CHECK-FIRST-NOT: spir_func void @func.vector() {{.*}} !sycl_used_aspects +; CHECK-SECOND: spir_func void @func.vector() !sycl_used_aspects ![[MDID]] { +define spir_func void @func.vector() { + %tmp = alloca <4 x double> + ret void +} + +; CHECK-FIRST-NOT: spir_func void @func.composite() {{.*}} !sycl_used_aspects +; CHECK-SECOND: spir_func void @func.composite() !sycl_used_aspects ![[MDID]] { +define spir_func void @func.composite() { + %tmp = alloca %composite + ret void +} + +!sycl_aspects = !{!0} +!0 = !{!"fp64", i32 6} + +; CHECK-SECOND: ![[MDID]] = !{i32 6} diff --git a/llvm/test/SYCLLowerIR/PropagateAspectsUsage/exclude-aspect.ll b/llvm/test/SYCLLowerIR/PropagateAspectsUsage/exclude-aspect.ll new file mode 100644 index 0000000000000..59c7964cd60ad --- /dev/null +++ b/llvm/test/SYCLLowerIR/PropagateAspectsUsage/exclude-aspect.ll @@ -0,0 +1,139 @@ +; RUN: opt -passes=sycl-propagate-aspects-usage -sycl-propagate-aspects-usage-exclude-aspects=aspect4,aspect1 -S < %s | FileCheck %s +; +; Test checks that the pass is able to collect all aspects used in a function + +%A = type { i32 } +%B = type { i32 } +%C = type { i32 } +%D = type { i32 } + +; None of funcA's aspects are excluded. +; CHECK: define spir_func void @funcA() !sycl_used_aspects ![[#ID0:]] { +define spir_func void @funcA() { + %tmp = alloca %A + ret void +} + +; funcB uses "aspect1" which is excluded, so the resulting aspects are the same +; as for funcA. +; CHECK: define spir_func void @funcB() !sycl_used_aspects ![[#ID0]] { +define spir_func void @funcB() { + %tmp = alloca %B + call spir_func void @funcA() + ret void +} + +; funcC has an aspect excluded, propagated from funcB. +; CHECK: define spir_func void @funcC() !sycl_used_aspects ![[#ID1:]] { +define spir_func void @funcC() { + %tmp = alloca %C + call spir_func void @funcB() + ret void +} + +; funcD has two aspects excluded; one from the use of D and one from propagated. +; from funcB and funcC. +; CHECK: define spir_func void @funcD() !sycl_used_aspects ![[#ID2:]] { +define spir_func void @funcD() { + %tmp = alloca %D + call spir_func void @funcC() + ret void +} + +; kernel1 has the same aspects as funcD. +; CHECK: define spir_kernel void @kernel1() !sycl_used_aspects ![[#ID2]] +define spir_kernel void @kernel1() { + call spir_func void @funcD() + ret void +} + +; funcE should get none of its explicitly declared aspects in its +; sycl_used_aspects +; CHECK: define spir_func void @funcE() !sycl_declared_aspects ![[#DA1:]] { +define spir_func void @funcE() !sycl_declared_aspects !10 { + ret void +} + +; funcF should have the same aspects as funcE +; CHECK-NOT: define spir_func void @funcF() {{.*}} !sycl_used_aspects +define spir_func void @funcF() { + call spir_func void @funcE() + ret void +} + +; funcG only keeps one aspect, the rest are excluded +; CHECK: define spir_func void @funcG() !sycl_declared_aspects ![[#DA2:]] !sycl_used_aspects ![[#ID3:]] +define spir_func void @funcG() !sycl_declared_aspects !11 { + ret void +} + +; funcH should have the same aspects as funcG +; CHECK: define spir_func void @funcH() !sycl_used_aspects ![[#ID3]] +define spir_func void @funcH() { + call spir_func void @funcG() + ret void +} + +; CHECK: define spir_kernel void @kernel2() !sycl_used_aspects ![[#ID3]] +define spir_kernel void @kernel2() { + call spir_func void @funcF() + call spir_func void @funcH() + ret void +} + +; CHECK: define spir_func void @funcI() !sycl_used_aspects ![[#DA1]] { +define spir_func void @funcI() !sycl_used_aspects !10 { + ret void +} + +; CHECK-NOT: define spir_func void @funcJ() {{.*}} !sycl_used_aspects +define spir_func void @funcJ() { + call spir_func void @funcI() + ret void +} + +; +; Note that the listed aspects can be reordered due to the merging of the +; aspect sets. +; CHECK: define spir_func void @funcK() !sycl_used_aspects ![[#ID4:]] { +define spir_func void @funcK() !sycl_used_aspects !11 { + ret void +} + +; CHECK: define spir_func void @funcL() !sycl_used_aspects ![[#ID3]] +define spir_func void @funcL() { + call spir_func void @funcK() + ret void +} + +; CHECK: define spir_kernel void @kernel3() !sycl_used_aspects ![[#ID3]] +define spir_kernel void @kernel3() { + call spir_func void @funcK() + call spir_func void @funcL() + ret void +} + +!sycl_types_that_use_aspects = !{!0, !1, !2, !3} +!0 = !{!"A", i32 0} +!1 = !{!"B", i32 1} +!2 = !{!"C", i32 2} +!3 = !{!"D", i32 3, i32 4} + +!sycl_aspects = !{!4, !5, !6, !7, !8, !9} +!4 = !{!"aspect0", i32 0} +!5 = !{!"aspect1", i32 1} +!6 = !{!"aspect2", i32 2} +!7 = !{!"aspect3", i32 3} +!8 = !{!"aspect4", i32 4} +!9 = !{!"fp64", i32 5} + +!10 = !{i32 1} +!11 = !{i32 4, i32 2, i32 1} +; CHECK-DAG: ![[#DA1]] = !{i32 1} +; CHECK-DAG: ![[#DA2]] = !{i32 4, i32 2, i32 1} + +; CHECK-DAG: ![[#ID0]] = !{i32 0} +; CHECK-DAG: ![[#ID1]] = !{i32 2, i32 0} +; CHECK-DAG: ![[#ID2]] = !{i32 0, i32 2, i32 3} +; CHECK-DAG: ![[#ID3]] = !{i32 2} +; CHECK-DAG: ![[#ID4]] = !{i32 2, i32 4, i32 1} From 186b69a0ab9799a729e43bdac99bc317fe18266e Mon Sep 17 00:00:00 2001 From: "Larsen, Steffen" Date: Tue, 21 Mar 2023 11:57:39 -0700 Subject: [PATCH 2/2] Add comment to exclusion argument Signed-off-by: Larsen, Steffen --- clang/lib/CodeGen/BackendUtil.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/clang/lib/CodeGen/BackendUtil.cpp b/clang/lib/CodeGen/BackendUtil.cpp index 95a89a0a6f64d..47b9d4e39f1a6 100644 --- a/clang/lib/CodeGen/BackendUtil.cpp +++ b/clang/lib/CodeGen/BackendUtil.cpp @@ -911,7 +911,8 @@ void EmitAssemblyHelper::RunOptimizationPipeline( PB.registerPipelineStartEPCallback( [&](ModulePassManager &MPM, OptimizationLevel Level) { MPM.addPass(ESIMDVerifierPass(LangOpts.SYCLESIMDForceStatelessMem)); - MPM.addPass(SYCLPropagateAspectsUsagePass({"fp64"})); + MPM.addPass( + SYCLPropagateAspectsUsagePass(/*ExcludeAspects=*/{"fp64"})); }); // Add the InferAddressSpaces pass for all the SPIR[V] targets @@ -1027,7 +1028,8 @@ void EmitAssemblyHelper::RunOptimizationPipeline( MPM.addPass(DeadArgumentEliminationSYCLPass()); // Rerun aspect propagation without warning diagnostics. - MPM.addPass(SYCLPropagateAspectsUsagePass({}, /*ValidateAspects=*/false)); + MPM.addPass(SYCLPropagateAspectsUsagePass(/*ExcludeAspects=*/{}, + /*ValidateAspects=*/false)); // Add SPIRITTAnnotations pass to the pass manager if // -fsycl-instrument-device-code option was passed. This option can be