[SYCL] Add support to propagate compile flags to device backend compiler #8763

asudarsa · 2023-03-24T05:48:30Z

This change is a second attempt to add this support. An earlier attempt was here: #7373

In this change, following changes have been made:

clang changes to add a new function attribute: sycl-optlevel
sycl-post-link changes to process this attribute, split modules based on optimization level, and add a new property named 'optLevel' to SYCL/misc properties' property set.
SYCL runtime and plugin changes to access this device image property and propagate a backend specific optimization flag to the backend compiler.
Documentation
2 unit tests and 1 e2e test

Thanks

Signed-off-by: Arvind Sudarsanam <[email protected]>

…ort_for_compile_flag_propagate_try_2

asudarsa · 2023-03-24T05:51:07Z

llvm/test/tools/sycl-post-link/device-code-split/per-aspect-split-1.ll

@@ -10,29 +10,29 @@
 ; RUN: sycl-post-link -split=auto -symbols -S < %s -o %t.table
 ; RUN: FileCheck %s -input-file=%t_0.ll --check-prefixes CHECK-M0-IR \
 ; RUN: --implicit-check-not kernel0 --implicit-check-not kernel1
-; RUN: FileCheck %s -input-file=%t_1.ll --check-prefixes CHECK-M1-IR \
+; RUN: FileCheck %s -input-file=%t_2.ll --check-prefixes CHECK-M1-IR \


These test updates were triggered by the fact that the order of the entries in the .table file changed due to the addition of the new entry in the list optional kernel features. A better way to NOT depend on the order of the entries is required in the long run.

FYI: that unstable ordering is going to be fixed in #8833. That PR will also allow to simplify your changes to device code split down to a single line of code

sycl/source/detail/program_manager/program_manager.cpp

bader · 2023-03-24T05:55:09Z

This change is a second attempt to add this support. An earlier attempt was here: #7373

Please, keep using #7373 to preserve review history.

asudarsa · 2023-03-24T06:00:52Z

An E2E test is added here: intel/llvm-test-suite#1691

Thanks

Signed-off-by: Arvind Sudarsanam <[email protected]>

asudarsa · 2023-03-24T06:20:18Z

This change is a second attempt to add this support. An earlier attempt was here: #7373

Please, keep using #7373 to preserve review history.

Hi @bader

This PR is a major revamp. Adding this on top of #7373 might end up confusing new reviewers. Alexey Sachkov is the main reviewer so far and we did discuss this.

Thanks

llvm/test/tools/sycl-post-link/sycl-opt-level.ll

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

sycl/source/detail/program_manager/program_manager.cpp

sarnex · 2023-03-24T14:52:50Z

sycl/source/detail/program_manager/program_manager.cpp

+  } else if (Plugin.getBackend() == backend::opencl) {
+    if (!CompileOpts.empty() && (optLevel == 0))
+      CompileOpts += " ";
+    if (optLevel == 0)


so for opt > 0 we ignore it for ocl? should we at least warn?

Added warning message for cases where backend does not provide optimization option. Thanks

sycl/doc/design/PropagateCompilerFlagsToRuntime.md

sarnex · 2023-03-24T14:59:06Z

sycl/doc/design/PropagateCompilerFlagsToRuntime.md

+| Front-end option | L0 backend option | OpenCL backend option |
+| ---------------- | ----------------- | --------------------- |
+|      -O0         |  -ze-opt-disable  |   -cl-opt-disable     |
+|      -O1         |  -ze-opt-level=1  |   /* no option */     |


i commented this somewhere else but silently ignoring it seems confusing for the user, i recommend a warning

We should be really careful with adding such warning. The problem here is that we can't suggest anything to user to fix this. There is literally no equivalent for different optimization levels in OpenCL: you either pass no flags, which means compiler optimizes as it wants to; or you pass -cl-opt-disable, which mean no optimizations at all.

-O2 is a default level of optimizations and we definitely don't want to have warning with no (!) workaround emitted for almost all existing applications.

Unless we can suggest a way to fix (in user code/build script) and not just suppress the warning, it shouldn't be added, I believe.

However, it makes sense to add a section describing this behavior into our documentation somewhere

Hi @AlexeySachkov

Thanks for detailed input on this. I am currently letting the users know about this via a warning. ""Optimization level not propagated to backend"; Do you think this is something which might confuse the user?
Do you want me to add the information in the design doc here?

Thanks again.

@AlexeySachkov good take, thanks

Thanks for detailed input on this. I am currently letting the users know about this via a warning. ""Optimization level not propagated to backend"; Do you think this is something which might confuse the user?

I think any warning should be somehow resolvable by doing something other than -Wno-this-particular-warning (i.e. suppressing it). One possible pain point is -Werror configured on customer side. Therefore, I would recommend not to add the warning at all.

Do you want me to add the information in the design doc here?

In my last sentence about documentation I mean some user-facing documentation. Just so that our customers are aware about implementation details and can build their expectations accordingly.

sycl/doc/design/PropagateCompilerFlagsToRuntime.md

clang/test/CodeGenSYCL/sycl-add-opt-level-attrib.cpp

Signed-off-by: Arvind Sudarsanam <[email protected]>

bso-intel · 2023-04-13T16:56:04Z

#9008 - Basic/memory_consumption.cpp fail is a known issue.

I don't think this failure is still there with your latest test.

asudarsa · 2023-04-13T17:08:55Z

#9008 - Basic/memory_consumption.cpp fail is a known issue.

I don't think this failure is still there with your latest test.

yes. I added this comment based on the previous run. Thanks

intel#8763 Signed-off-by: Jaime Arteaga <[email protected]>

asudarsa added 6 commits March 20, 2023 15:21

Add frontend changes

fb3bebd

Signed-off-by: Arvind Sudarsanam <[email protected]>

Adding support to propagate frontend compile time options to backend

d3a9e24

Signed-off-by: Arvind Sudarsanam <[email protected]>

Minor changes

395d550

Signed-off-by: Arvind Sudarsanam <[email protected]>

Modify order of split modules in the sycl-post-link table

0467da0

Signed-off-by: Arvind Sudarsanam <[email protected]>

Add documentation

1cc4d14

Signed-off-by: Arvind Sudarsanam <[email protected]>

Merge remote-tracking branch 'intel_llvm/sycl' into asudarsa/add_supp…

5c529d2

…ort_for_compile_flag_propagate_try_2

asudarsa requested review from a team as code owners March 24, 2023 05:48

asudarsa requested a review from bso-intel March 24, 2023 05:48

asudarsa commented Mar 24, 2023

View reviewed changes

sycl/source/detail/program_manager/program_manager.cpp Outdated Show resolved Hide resolved

asudarsa mentioned this pull request Mar 24, 2023

Add test for SYCL optlevel compile flag propagation to runtime intel/llvm-test-suite#1691

Open

Fix formatting issues

570d720

Signed-off-by: Arvind Sudarsanam <[email protected]>

asudarsa temporarily deployed to aws March 24, 2023 06:37 — with GitHub Actions Inactive

asudarsa temporarily deployed to aws March 24, 2023 07:08 — with GitHub Actions Inactive

sarnex reviewed Mar 24, 2023

View reviewed changes

premanandrao reviewed Mar 24, 2023

View reviewed changes

clang/test/CodeGenSYCL/sycl-add-opt-level-attrib.cpp Outdated Show resolved Hide resolved

Add plugin API to get backend option

992614e

Signed-off-by: Arvind Sudarsanam <[email protected]>

asudarsa requested review from a team as code owners March 27, 2023 16:23

asudarsa requested a review from sergey-semenov March 27, 2023 16:23

Address review comments on tests

e2e678a

Signed-off-by: Arvind Sudarsanam <[email protected]>

asudarsa temporarily deployed to aws March 27, 2023 17:29 — with GitHub Actions Inactive

bader merged commit f45fb51 into intel:sycl Apr 13, 2023

This was referenced Apr 20, 2023

Add kernel property to propagate compile options to backend #7373

Closed

[SYCL][NFCI] Refactor device code split implementation once again #8833

Merged

jandres742 pushed a commit to jandres742/llvm that referenced this pull request Apr 21, 2023

Port Add support to propagate compile flags to device backend compiler

b6a6233

intel#8763 Signed-off-by: Jaime Arteaga <[email protected]>

jandres742 pushed a commit to jandres742/llvm that referenced this pull request Apr 24, 2023

Port Add support to propagate compile flags to device backend compiler

4b02009

intel#8763 Signed-off-by: Jaime Arteaga <[email protected]>

jandres742 pushed a commit to jandres742/llvm that referenced this pull request May 3, 2023

Port Add support to propagate compile flags to device backend compiler

096d20a

intel#8763 Signed-off-by: Jaime Arteaga <[email protected]>

jandres742 pushed a commit to jandres742/llvm that referenced this pull request May 16, 2023

Port Add support to propagate compile flags to device backend compiler

ecb80db

intel#8763 Signed-off-by: Jaime Arteaga <[email protected]>

jandres742 pushed a commit to jandres742/llvm that referenced this pull request May 23, 2023

Port Add support to propagate compile flags to device backend compiler

1c39dd7

intel#8763 Signed-off-by: Jaime Arteaga <[email protected]>

jandres742 pushed a commit to jandres742/llvm that referenced this pull request May 26, 2023

Port Add support to propagate compile flags to device backend compiler

7250d46

intel#8763 Signed-off-by: Jaime Arteaga <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SYCL] Add support to propagate compile flags to device backend compiler #8763

[SYCL] Add support to propagate compile flags to device backend compiler #8763

asudarsa commented Mar 24, 2023 •

edited

Loading

asudarsa Mar 24, 2023

AlexeySachkov Apr 3, 2023

bader commented Mar 24, 2023

asudarsa commented Mar 24, 2023

asudarsa commented Mar 24, 2023

sarnex Mar 24, 2023

asudarsa Mar 27, 2023

sarnex Mar 24, 2023

AlexeySachkov Mar 28, 2023

asudarsa Mar 29, 2023

sarnex Mar 29, 2023

AlexeySachkov Apr 3, 2023

bso-intel commented Apr 13, 2023

asudarsa commented Apr 13, 2023

[SYCL] Add support to propagate compile flags to device backend compiler #8763

[SYCL] Add support to propagate compile flags to device backend compiler #8763

Conversation

asudarsa commented Mar 24, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bader commented Mar 24, 2023

asudarsa commented Mar 24, 2023

asudarsa commented Mar 24, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bso-intel commented Apr 13, 2023

asudarsa commented Apr 13, 2023

asudarsa commented Mar 24, 2023 •

edited

Loading