[SYCL] Add new auto device code split mode #2827

AlexeySachkov · 2020-11-27T13:55:50Z

This patch introduces new device code split mode auto, which is
intended to automatically select the best device code split mode and
apply it.

At the moment, auto is equivalent to per_source for most cases and
it is equivalent to off in case of precense of function pointers.

AlexeySachkov · 2020-11-27T13:56:06Z

/summary:run

AGindinson · 2020-11-27T16:59:26Z

I would argue that the current title is somewhat misleading: before revising the actual changes, I've mistaken this for "always run the split unless -fsycl-device-code-split=off is specified, even if no option was passed at all". Maybe this variant would be a tad cleaner?

[SYCL] Enable a default mode for device code split

Fznamznon

Let's also add tests.

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

AlexeySachkov · 2020-12-03T09:40:36Z

/summary:run

AGindinson

The driver part LGTM.

This patch introduces new device code split mode `auto`, which is intended to automatically select the best device code split mode and apply it. At the moment, `auto` is equivalent to `per_source` for most cases and it is equivalent to `off` in case of precense of function pointers.

Added LIT tests. Update heuristic to disable device code split when indirect calls are present in the input module.

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

kbobrovs · 2020-12-23T03:26:17Z

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

@@ -289,6 +291,42 @@ enum KernelMapEntryScope {
  Scope_Global     // single entry in the map for all kernels
 };

+static KernelMapEntryScope selectDeviceCodeSplitScopeAutomatically(Module &M) {
+  // Here we can employ various heuristics to decide which way to split kernels


There are more comments then the code here :) I think the idea is pretty straightforward and the description could be shrinked.

I've re-structured comments a bit in edfa44c

kbobrovs · 2020-12-23T03:27:20Z

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

-  if (IROutputOnly && DoSplit) {
-    errs() << "error: -" << SplitMode.ArgStr << " can't be used with -"
-           << IROutputOnly.ArgStr << "\n";
+  if (IROutputOnly && (DoSplit && SplitMode != SPLIT_AUTO)) {


Why SPLIT_AUTO is allowed with IR output only?

Why SPLIT_AUTO is allowed with IR output only?

Technically this is me being a bit lazy to dig further into the driver and do some changes there. Basically, there are compilation flows (related to AOT), where sycl-post-link is involved with -ir-output-only option and according to my changes, -split= is almost always passed to sycl-post-link (unless split was explicitly disabled) and it triggered errors in some LIT tests.

OK, so the auto heuristics in presence of -ir-output-only is fixed to "don't split" - ? please add a comment then, to user-visible description too

OK, so the auto heuristics in presence of -ir-output-only is fixed to "don't split" - ? please add a comment then, to user-visible description too

That's correct, updated help message of the tool in edfa44c

clang/include/clang/Driver/Options.td

Co-authored-by: Artem Gindinson <artem.gindinson@intel.com>

AGindinson

The driver part LGTM.

Since `auto` value introduction for `-fsycl-device-code-split` in commit 184d258 (intel#2827), it stands as the default code split mechanism. Signed-off-by: Artem Gindinson <artem.gindinson@intel.com>

Since `auto` value introduction for `-fsycl-device-code-split` in commit 184d258 (#2827), it stands as the default code split mechanism. Signed-off-by: Artem Gindinson <artem.gindinson@intel.com>

Since `auto` value introduction for `-fsycl-device-code-split` in commit 184d258 (intel#2827), it stands as the default code split mechanism. Signed-off-by: Artem Gindinson <artem.gindinson@intel.com>

This improves SPV_KHR_untyped_pointers extension. Removing struct type from special handling (translate as typed pointer) allowed to fix `spirv-val` error in `CXX/global-ctor.cl` test: ``` error: line 88: OpFunctionCall Argument <id> '25[%this1]'s type does not match Function <id> '11[%_ptr_Generic_class_Something]'s parameter type. %30 = OpFunctionCall %void %_ZNU3AS49SomethingC2Ei %this1 %26 ``` Other changes allow to translate structs in a new way without violating validation or test checks. Original commit: KhronosGroup/SPIRV-LLVM-Translator@15fd1cc50e12465

AlexeySachkov requested review from Fznamznon and AGindinson November 27, 2020 13:55

Fznamznon reviewed Nov 30, 2020

View reviewed changes

llvm/tools/sycl-post-link/sycl-post-link.cpp Outdated Show resolved Hide resolved

llvm/tools/sycl-post-link/sycl-post-link.cpp Outdated Show resolved Hide resolved

AGindinson previously approved these changes Dec 9, 2020

View reviewed changes

AlexeySachkov added 3 commits December 22, 2020 11:00

Allow -split=auto together with -ir-output-only

d29970f

Improve implementation and tests

73faebb

Added LIT tests. Update heuristic to disable device code split when indirect calls are present in the input module.

AlexeySachkov dismissed AGindinson’s stale review via 73faebb December 22, 2020 11:52

AlexeySachkov force-pushed the private/asachkov/auto-device-code-split branch from 4175932 to 73faebb Compare December 22, 2020 11:52

AlexeySachkov changed the title ~~[SYCL] Enable device code split by default~~ [SYCL] Add new auto device code split mode Dec 22, 2020

AlexeySachkov marked this pull request as ready for review December 22, 2020 11:54

AlexeySachkov requested review from bader, kbobrovs and mdtoguchi as code owners December 22, 2020 11:54

kbobrovs reviewed Dec 23, 2020

View reviewed changes

AlexeySachkov added 2 commits December 24, 2020 12:30

Apply comments

edfa44c

Fix clang-format

d374ffe

AlexeySachkov requested review from AGindinson and kbobrovs December 24, 2020 14:58

AGindinson reviewed Dec 24, 2020

View reviewed changes

clang/include/clang/Driver/Options.td Outdated Show resolved Hide resolved

clang/include/clang/Driver/Options.td Outdated Show resolved Hide resolved

Apply suggestions from code review

6441267

Co-authored-by: Artem Gindinson <artem.gindinson@intel.com>

AGindinson approved these changes Dec 24, 2020

View reviewed changes

kbobrovs approved these changes Dec 24, 2020

View reviewed changes

romanovvlad merged commit 184d258 into intel:sycl Dec 25, 2020

AlexeySachkov mentioned this pull request Dec 29, 2020

Avoid CL_BUILD_PROGRAM_FAILURE under DPC++ for devices not supporting doubles #2968

Closed

AlexeySachkov deleted the private/asachkov/auto-device-code-split branch February 25, 2021 12:27

AGindinson mentioned this pull request Sep 7, 2021

[SYCL][NFC] Update comment on default code split behavior #4509

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[SYCL] Add new auto device code split mode #2827

[SYCL] Add new auto device code split mode #2827

Uh oh!

AlexeySachkov commented Nov 27, 2020 •

edited

Loading

Uh oh!

AlexeySachkov commented Nov 27, 2020

Uh oh!

AGindinson commented Nov 27, 2020 •

edited

Loading

Uh oh!

Fznamznon left a comment

Uh oh!

Uh oh!

Uh oh!

AlexeySachkov commented Dec 3, 2020

Uh oh!

AGindinson left a comment

Uh oh!

Uh oh!

kbobrovs Dec 23, 2020

Uh oh!

AlexeySachkov Dec 24, 2020

Uh oh!

kbobrovs Dec 23, 2020

Uh oh!

AlexeySachkov Dec 23, 2020

Uh oh!

kbobrovs Dec 24, 2020

Uh oh!

AlexeySachkov Dec 24, 2020

Uh oh!

Uh oh!

Uh oh!

AGindinson left a comment

Uh oh!

Uh oh!

[SYCL] Add new auto device code split mode #2827

[SYCL] Add new auto device code split mode #2827

Uh oh!

Conversation

AlexeySachkov commented Nov 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AlexeySachkov commented Nov 27, 2020

Uh oh!

AGindinson commented Nov 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Fznamznon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

AlexeySachkov commented Dec 3, 2020

Uh oh!

AGindinson left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kbobrovs Dec 23, 2020

Choose a reason for hiding this comment

Uh oh!

AlexeySachkov Dec 24, 2020

Choose a reason for hiding this comment

Uh oh!

kbobrovs Dec 23, 2020

Choose a reason for hiding this comment

Uh oh!

AlexeySachkov Dec 23, 2020

Choose a reason for hiding this comment

Uh oh!

kbobrovs Dec 24, 2020

Choose a reason for hiding this comment

Uh oh!

AlexeySachkov Dec 24, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

AGindinson left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

AlexeySachkov commented Nov 27, 2020 •

edited

Loading

AGindinson commented Nov 27, 2020 •

edited

Loading