[SYCL] Add generation of device image with specialization constants replaced by default values #10115

maksimsab · 2023-06-28T14:53:08Z

No description provided.

…eplaced by default values

llvm/test/tools/sycl-post-link/spec-constants/default-value/esimd.ll

llvm/tools/sycl-post-link/sycl-post-link.cpp

llvm/test/tools/sycl-post-link/spec-constants/default-value/device-image.ll

llvm/tools/sycl-post-link/ModuleSplitter.cpp

llvm/tools/sycl-post-link/sycl-post-link.cpp

llvm/tools/sycl-post-link/SpecConstants.cpp

maksimsab · 2023-07-13T13:31:24Z

againull · 2023-07-27T18:16:02Z

llvm/tools/sycl-post-link/ModuleSplitter.cpp

+ModuleDesc CloneModuleDesc(const ModuleDesc &MD) {
+  std::unique_ptr<Module> NewModule = CloneModule(MD.getModule());
+  ModuleDesc NewMD(std::move(NewModule));
+  NewMD.EntryPoints.Props = MD.EntryPoints.Props;
+  return NewMD;
+}


It seems like some kind of deep copy of ModuleDesc is needed here. At least in addition to EntryPoints.Props also EntryPoints.Functions need to be copied.
Otherwise cloned module/device image is not registered in ProgramManager through void ProgramManager::addImages(pi_device_binaries DeviceBinary) because it doesn't have entry points.

Addresses it. Now symbol generation is being checked in llvm/test/tools/sycl-post-link/spec-constants/default-value/device-image.ll

llvm/tools/sycl-post-link/sycl-post-link.cpp

llvm/tools/sycl-post-link/SpecConstants.h

llvm/tools/sycl-post-link/SpecConstants.cpp

llvm/tools/sycl-post-link/ModuleSplitter.h

llvm/test/tools/sycl-post-link/spec-constants/default-value/struct-with-padding.ll

llvm/tools/sycl-post-link/sycl-post-link.cpp

llvm/tools/sycl-post-link/SpecConstants.cpp

llvm/test/tools/sycl-post-link/spec-constants/default-value/split-by-source.ll

llvm/test/tools/sycl-post-link/spec-constants/default-value/esimd.ll

llvm/test/tools/sycl-post-link/spec-constants/default-value/device-image.ll

llvm/test/tools/sycl-post-link/spec-constants/default-value/split-by-kernel.ll

llvm/test/tools/sycl-post-link/spec-constants/default-value/struct-with-padding.ll

AlexeySachkov · 2023-08-16T08:59:21Z

llvm/tools/sycl-post-link/SpecConstants.cpp

 #include "llvm/IR/InstIterator.h"
 #include "llvm/IR/Instruction.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/Operator.h"

+#include <vector>


Unnecessary include

I saw there 2 usages of std::vector.

Got it, I just though that we don't use it at all

AlexeySachkov · 2023-08-16T09:08:14Z

llvm/tools/sycl-post-link/SpecConstants.cpp

+  if (Mode == HandlingMode::default_values)
+    for (Function *F : SpecConstDeclarations)
+      F->removeFromParent();


I still don't think that removal of unused declarations should be a part of SpecConstantsPass, because any transformation pass could leave those and having cleanup code in each of them does not seem like a good design. We have a dedicated pass for cleanup, no need to duplicate its code to every pass.

You can just add StripDeadPrototypesPass to the pipeline when launching SpecConstantsPass to emit that extra device image with default values of spec constants

maksimsab changed the title ~~[SYCL] Add generation of device image with specialization constants replaced by default values~~ [SYCL][NFC] Add generation of device image with specialization constants replaced by default values Jun 28, 2023

maksimsab changed the title ~~[SYCL][NFC] Add generation of device image with specialization constants replaced by default values~~ [SYCL] Add generation of device image with specialization constants replaced by default values Jun 29, 2023

maksimsab force-pushed the add_devimage_spec_consts branch 6 times, most recently from 2d6a440 to e2c6b23 Compare July 12, 2023 14:09

[SYCL] Add generation of device image with specialization constants r…

c4c9bdb

…eplaced by default values

maksimsab force-pushed the add_devimage_spec_consts branch from e2c6b23 to c4c9bdb Compare July 12, 2023 14:12

maksimsab marked this pull request as ready for review July 12, 2023 14:13

maksimsab requested a review from a team as a code owner July 12, 2023 14:13

maksimsab temporarily deployed to aws July 12, 2023 14:31 — with GitHub Actions Inactive

maksimsab temporarily deployed to aws July 12, 2023 15:11 — with GitHub Actions Inactive

AlexeySachkov requested changes Jul 13, 2023

View reviewed changes

maksimsab added 2 commits July 25, 2023 09:35

[SYCL] address remarks from code review

30c5715

[SYCL] apply clang-format

cfaae20

maksimsab temporarily deployed to aws July 25, 2023 16:59 — with GitHub Actions Inactive

maksimsab temporarily deployed to aws July 25, 2023 17:36 — with GitHub Actions Inactive

[SYCL] add handling of scalar types and padded structures

9e2dc55

maksimsab temporarily deployed to aws July 27, 2023 16:38 — with GitHub Actions Inactive

maksimsab temporarily deployed to aws July 27, 2023 17:22 — with GitHub Actions Inactive

againull reviewed Jul 27, 2023

View reviewed changes

maksimsab added 2 commits July 28, 2023 04:40

Merge branch 'sycl' into add_devimage_spec_consts

b4dc7df

[SYCL] Enable test

9debaa7

maksimsab requested a review from AlexeySachkov July 28, 2023 11:59

maksimsab temporarily deployed to aws July 28, 2023 12:19 — with GitHub Actions Inactive

maksimsab temporarily deployed to aws July 28, 2023 12:59 — with GitHub Actions Inactive

AlexeySachkov reviewed Jul 28, 2023

View reviewed changes

[SYCL] address code review remarks

1c94f5f

maksimsab temporarily deployed to aws August 3, 2023 13:28 — with GitHub Actions Inactive

maksimsab temporarily deployed to aws August 3, 2023 14:08 — with GitHub Actions Inactive

[SYCL] fix symbols generation

23f006d

maksimsab temporarily deployed to aws August 3, 2023 14:59 — with GitHub Actions Inactive

maksimsab temporarily deployed to aws August 3, 2023 15:40 — with GitHub Actions Inactive

maksimsab added 3 commits August 4, 2023 07:50

Merge branch 'sycl' into add_devimage_spec_consts

c0b75bb

[SYCL] Incorporate logic for default values into the general loop

a03754a

[SYCL] generate symbols more correctly

e69a7f6

maksimsab temporarily deployed to aws August 4, 2023 15:53 — with GitHub Actions Inactive

maksimsab temporarily deployed to aws August 4, 2023 16:42 — with GitHub Actions Inactive

maksimsab added 2 commits August 7, 2023 06:02

[SYCL] refactor function

a561166

Merge branch 'sycl' into add_devimage_spec_consts

7697e7b

maksimsab requested a review from AlexeySachkov August 7, 2023 13:04

maksimsab temporarily deployed to aws August 7, 2023 13:19 — with GitHub Actions Inactive

maksimsab temporarily deployed to aws August 7, 2023 14:31 — with GitHub Actions Inactive

AlexeySachkov reviewed Aug 8, 2023

View reviewed changes

maksimsab added 3 commits August 8, 2023 07:56

[SYCL] polish PR

7af4749

[SYCL] Add handling of bool specialization constant

f4fc91c

[SYCL] fix help.test

12adaaa

maksimsab requested a review from AlexeySachkov August 10, 2023 12:32

maksimsab added 2 commits August 11, 2023 07:32

Merge branch 'sycl' into add_devimage_spec_consts

1d6a55a

[SYCL] adjust tests to parameter renaming

a285f2c

AlexeySachkov approved these changes Aug 16, 2023

View reviewed changes

[SYCL] add StripDeadPrototypesPass after SpecConstantPass

b6c056f

AlexeySachkov merged commit a7ef268 into intel:sycl Aug 17, 2023

[SYCL] Add generation of device image with specialization constants replaced by default values #10115

[SYCL] Add generation of device image with specialization constants replaced by default values #10115

Uh oh!

Conversation

maksimsab commented Jun 28, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

maksimsab commented Jul 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

againull Jul 27, 2023

Choose a reason for hiding this comment

Uh oh!

maksimsab Aug 4, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AlexeySachkov Aug 16, 2023

Choose a reason for hiding this comment

Uh oh!

maksimsab Aug 17, 2023

Choose a reason for hiding this comment

Uh oh!

AlexeySachkov Aug 17, 2023

Choose a reason for hiding this comment

Uh oh!

AlexeySachkov Aug 16, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

maksimsab commented Jul 13, 2023 •

edited

Loading