-
Notifications
You must be signed in to change notification settings - Fork 796
[SYCL] Add dummy image generation for virtual functions #15942
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: sycl
Are you sure you want to change the base?
Changes from 1 commit
066bd18
befd24b
3713fae
94d63ff
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| ; RUN: sycl-post-link -split=auto -properties -S < %s -o %t.table | ||
| ; RUN: FileCheck %s --input-file=%t.table --check-prefix=CHECK-TABLE | ||
| ; RUN: FileCheck %s --input-file=%t_0.ll --check-prefix=CHECK-FP64-SPLIT | ||
| ; RUN: FileCheck %s --input-file=%t_1.ll --check-prefix=CHECK-FP64-DUMMY | ||
| ; RUN: FileCheck %s --input-file=%t_1.prop --check-prefix=CHECK-FP64-DUMMY-PROPS | ||
| ; RUN: FileCheck %s --input-file=%t_2.ll --check-prefix=CHECK-FP32-SPLIT | ||
|
|
||
| ; CHECK-TABLE: _0.prop | ||
| ; CHECK-TABLE-NEXT: _1.prop | ||
| ; CHECK-TABLE-NEXT: _2.prop | ||
|
|
||
| ; CHECK-FP64-SPLIT: define spir_func void @bar() | ||
| ; CHECK-FP32-SPLIT: define spir_func void @foo() | ||
|
|
||
| ; CHECK-FP64-DUMMY: define spir_func void @bar() | ||
| ; CHECK-FP64-DUMMY-NEXT: entry: | ||
| ; CHECK-FP64-DUMMY-NEXT: ret void | ||
|
|
||
| ; CHECK-FP64-DUMMY-PROPS: dummy=1 | ||
|
|
||
| define spir_func void @foo() #1 { | ||
| %x = alloca float | ||
| ret void | ||
| } | ||
|
|
||
| define spir_func void @bar() #1 !sycl_used_aspects !1 { | ||
| %x = alloca double | ||
| %d = load double, ptr %x | ||
| %res = fadd double %d, %d | ||
| ret void | ||
| } | ||
|
|
||
| attributes #1 = { "sycl-module-id"="v.cpp" "indirectly-callable"="setA" } | ||
|
|
||
| !sycl_aspects = !{!0} | ||
| !0 = !{!"fp64", i32 6} | ||
| !1 = !{i32 6} | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -306,7 +306,8 @@ std::string saveModuleIR(Module &M, int I, StringRef Suff) { | |||||
|
|
||||||
| std::string saveModuleProperties(module_split::ModuleDesc &MD, | ||||||
| const GlobalBinImageProps &GlobProps, int I, | ||||||
| StringRef Suff, StringRef Target = "") { | ||||||
| StringRef Suff, StringRef Target = "", | ||||||
| bool IsDummy = false) { | ||||||
| auto PropSet = | ||||||
| computeModuleProperties(MD.getModule(), MD.entries(), GlobProps); | ||||||
|
|
||||||
|
|
@@ -318,6 +319,10 @@ std::string saveModuleProperties(module_split::ModuleDesc &MD, | |||||
| NewSuff += Target; | ||||||
| } | ||||||
|
|
||||||
| if (IsDummy) { | ||||||
| PropSet.add(PropSetRegTy::SYCL_VIRTUAL_FUNCTIONS, "dummy", 1); | ||||||
| } | ||||||
AlexeySachkov marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||
|
|
||||||
| std::error_code EC; | ||||||
| std::string SCFile = makeResultFileName(".prop", I, NewSuff); | ||||||
| raw_fd_ostream SCOut(SCFile, EC); | ||||||
|
|
@@ -416,7 +421,8 @@ void addTableRow(util::SimpleTable &Table, | |||||
| // IR component saving is skipped, and this file name is recorded as such in | ||||||
| // the result. | ||||||
| void saveModule(std::vector<std::unique_ptr<util::SimpleTable>> &OutTables, | ||||||
| module_split::ModuleDesc &MD, int I, StringRef IRFilename) { | ||||||
| module_split::ModuleDesc &MD, int I, StringRef IRFilename, | ||||||
| bool IsDummy = false) { | ||||||
| IrPropSymFilenameTriple BaseTriple; | ||||||
| StringRef Suffix = getModuleSuffix(MD); | ||||||
| MD.saveSplitInformationAsMetadata(); | ||||||
|
|
@@ -440,8 +446,8 @@ void saveModule(std::vector<std::unique_ptr<util::SimpleTable>> &OutTables, | |||||
| GlobalBinImageProps Props = {EmitKernelParamInfo, EmitProgramMetadata, | ||||||
| EmitExportedSymbols, EmitImportedSymbols, | ||||||
| DeviceGlobals}; | ||||||
| CopyTriple.Prop = | ||||||
| saveModuleProperties(MD, Props, I, Suffix, OutputFile.Target); | ||||||
| CopyTriple.Prop = saveModuleProperties(MD, Props, I, Suffix, | ||||||
| OutputFile.Target, IsDummy); | ||||||
| } | ||||||
| addTableRow(*Table, CopyTriple); | ||||||
| } | ||||||
|
|
@@ -741,6 +747,36 @@ bool isTargetCompatibleWithModule(const std::string &Target, | |||||
| return true; | ||||||
| } | ||||||
|
|
||||||
| std::optional<module_split::ModuleDesc> | ||||||
| makeDummy(module_split::ModuleDesc &MD) { | ||||||
| bool hasVirtualFunctions = false; | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| bool hasOptionalKernelFeatures = false; | ||||||
| for (Function &F : MD.getModule().functions()) { | ||||||
| if (F.hasFnAttribute("indirectly-callable")) | ||||||
| hasVirtualFunctions = true; | ||||||
| if (F.getMetadata("sycl_used_aspects")) | ||||||
| hasOptionalKernelFeatures = true; | ||||||
| if (hasVirtualFunctions && hasOptionalKernelFeatures) | ||||||
| break; | ||||||
| } | ||||||
| if (!hasVirtualFunctions || !hasOptionalKernelFeatures) | ||||||
| return {}; | ||||||
|
|
||||||
| auto MDCopy = MD.clone(); | ||||||
|
|
||||||
| for (Function &F : MDCopy.getModule().functions()) { | ||||||
| if (!F.hasFnAttribute("indirectly-callable")) | ||||||
| continue; | ||||||
|
|
||||||
| F.erase(F.begin(), F.end()); | ||||||
| BasicBlock *newBB = BasicBlock::Create(F.getContext(), "entry", &F); | ||||||
| IRBuilder<> builder(newBB); | ||||||
| builder.CreateRetVoid(); | ||||||
AlexeySachkov marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||
| } | ||||||
|
|
||||||
| return MDCopy; | ||||||
| } | ||||||
|
|
||||||
| std::vector<std::unique_ptr<util::SimpleTable>> | ||||||
| processInputModule(std::unique_ptr<Module> M) { | ||||||
| // Construct the resulting table which will accumulate all the outputs. | ||||||
|
|
@@ -893,6 +929,16 @@ processInputModule(std::unique_ptr<Module> M) { | |||||
|
|
||||||
| ++ID; | ||||||
| } | ||||||
|
|
||||||
| bool dummyEmitted = false; | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| for (module_split::ModuleDesc &IrMD : MMs) { | ||||||
| if (auto Dummy = makeDummy(IrMD)) { | ||||||
| saveModule(Tables, *Dummy, ID, OutIRFileName, /*IsDummy*/ true); | ||||||
| dummyEmitted = true; | ||||||
| } | ||||||
| } | ||||||
| if (dummyEmitted) | ||||||
| ++ID; | ||||||
|
Comment on lines
945
to
954
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We either need some comments here, or the function should be renamed (and likely we need both). For an unfamiliar reader it could be a mystery what "dummy" means and why do we need to make it for every device image we have produced. Also: do we have any information about device image carrying virtual function definitions in |
||||||
| } | ||||||
| return Tables; | ||||||
| } | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would also check that we have not emitted the property for other device images. And I would also throw a virtual function without any optional kernel features on it into the mix as well