[SYCL] Remove sycldevice target triple environment component #3929

bader · 2021-06-12T18:13:37Z

Today DPC++ compiler requires users to set "sycldevice" target triple component to separate SYCL compilation mode for SPIR target from other compilation mode (e.g. OpenCL).
Triple environment component is not intended to be used in such context and this patch removes sycldevice.

This patch also preserves backward compatibility with binaries built using old versions of the compiler, which enforced sycldevice environment component of the target triple. It's provided by clang-offload-bundler tool, which now implicitly looks for the additional SYCL offload kind bundle with a "legacy" triple.
A driver warning was added to let users know that sycldevice environment component is ignored now.

To simplify the review process, changes are split into multiple patches.
First nine patches updates the source code and three patches after that update target triples in llvm-spirv, llvm and clang project tests.
Commits after 46a928f address code review comments.

bader · 2021-06-12T18:53:27Z

/summary:run

bader · 2021-06-12T19:27:11Z

/summary:run

bader · 2021-06-12T20:41:52Z

TODO: add driver diagnostics about deprecated -sycldevice environment component.
TODO: keep supporting -sycldevice environment component explicitly set by users for backward compatibility

bader · 2021-07-22T15:47:11Z

TODO: keep supporting -sycldevice environment component explicitly set by users for backward compatibility

@mdtoguchi, @AGindinson, I'd like to know your opinion on the direction to implement this.

I see that when we link an object file, we unbundle it using following command:
clang-offload-bundler -type=o -targets=host-x86_64-unknown-linux-gnu,sycl-spir64-unknown-unknown -inputs=/tmp/lib.o -outputs=/tmp/lib-697d81.o,/tmp/lib-641a53.o -unbundle -allow-missing-bundles

Note, device triple doesn't have -sycldevice already, so if object file was built using old compiler, we won't get the right image.
My idea is when we have an object file as an input, triple target architecture is spir* and doesn't have -sycldevice environment component, we add one more triple to unbundler (with -sycldevice environment component) and corresponding output file(s). New output file(s) should be added to llvm-link command as well.

This should cover the simplest case when we link with an object file. I haven't looked into other scenarios yet, but I'd like to confirm that this idea sounds sane to you.

mdtoguchi · 2021-07-22T15:59:54Z

This should cover the simplest case when we link with an object file. I haven't looked into other scenarios yet, but I'd like to confirm that this idea sounds sane to you.

It does sound reasonable. Would there be a possibility of overlap of code (one previously built with -sycldevice and a new object without) that would both be linked in?

bader · 2021-07-22T16:15:08Z

This should cover the simplest case when we link with an object file. I haven't looked into other scenarios yet, but I'd like to confirm that this idea sounds sane to you.

It does sound reasonable. Would there be a possibility of overlap of code (one previously built with -sycldevice and a new object without) that would both be linked in?

I need to check what target tripe llvm-link uses for the module linked from spir-unknown-unknow and spir-unknown-unknow-sycldevice modules.
According to my understanding old compiler is able to produce device modules with -sycldevice environment component only.
I intent to make new compiler to always keep environment component unknown unless it's set to some other value by user. But user won't be able to set -sycldevice with a new compiler anyway.

`CheckSpirKernels` flag is enabled only in SYCL mode, so there is no need in checking triple for that.

It's not necessary.

Drop sycldevice component from the design document example. Drop sycldevice component from the matrix extension examples.

…ad kind check.

Add a workaround to clang-offload-bundler to support bundles produced by previous versions of the compiler

bader · 2021-08-05T17:22:19Z

This patch also preserves backward compatibility with binaries built using old versions of the compiler, which enforced sycldevice environment component of the target triple. It's provided by clang-offload-bundler tool, which now implicitly looks for the additional SYCL offload kind bundle with a "legacy" triple.

@vzakhari, does OpenMP compiler use the triple with sycldevice environment?

bader · 2021-08-05T17:25:01Z

This should cover the simplest case when we link with an object file. I haven't looked into other scenarios yet, but I'd like to confirm that this idea sounds sane to you.

It does sound reasonable. Would there be a possibility of overlap of code (one previously built with -sycldevice and a new object without) that would both be linked in?

@mdtoguchi, FYI.
I talked to @AGindinson and decided to take a different route and update clang-offload-bundler. Here is the change - 0b0317a. This seems to be much simpler than the driver changes adding another target for unbudling job.

vzakhari · 2021-08-05T17:27:03Z

This patch also preserves backward compatibility with binaries built using old versions of the compiler, which enforced sycldevice environment component of the target triple. It's provided by clang-offload-bundler tool, which now implicitly looks for the additional SYCL offload kind bundle with a "legacy" triple.

@vzakhari, does OpenMP compiler use the triple with sycldevice environment?

No, OpenMP compiler does not use sycldevice (or any other) environment. I believe it uses unknown.

clang/include/clang/Basic/DiagnosticDriverKinds.td

bader · 2021-08-05T19:04:23Z

/summary:run

bader · 2021-09-01T13:44:04Z

/summary:run

Recent changes in the DPC++ compiler (intel/llvm#3929) allow to disregard the `sycldevice` component from the target triple in command lines. From there on, producing IR files for tests should use the simplified target triple. Clean up the existing tests to match this change. Signed-off-by: Artem Gindinson <artem.gindinson@intel.com>

Recent changes in the DPC++ compiler (intel/llvm#3929) allow to disregard the `sycldevice` component from the target triple in command lines. From there on, IR files for the Translator tests should be produced using the simplified target triple. Clean up the existing tests to match this change. Signed-off-by: Artem Gindinson <artem.gindinson@intel.com>

AGindinson

LGTM.

Recent changes in the DPC++ compiler (intel/llvm#3929) allow to disregard the `sycldevice` component from the target triple in command lines. From there on, IR files for the Translator tests should be produced using the simplified target triple. Clean up the existing tests to match this change. Signed-off-by: Artem Gindinson <artem.gindinson@intel.com>

Recent changes in the DPC++ compiler (intel#3929) allow to disregard the `sycldevice` component from the target triple in command lines. From there on, IR files for the Translator tests should be produced using the simplified target triple. Clean up the existing tests to match this change. Signed-off-by: Artem Gindinson <artem.gindinson@intel.com> Original commit: KhronosGroup/SPIRV-LLVM-Translator@0a1f59c

With `-sycldevice` triple environment removal (as performed in intel#3929), backwards compatibility has been enforced for SYCL objects/object-format libraries only. To ensure that `.a` libraries (archives) built by older compiler versions are usable with newer compiler version, adjust `clang-offload-deps` to consider `-sycldevice`-marked symbols when encountering a SYCL target. Signed-off-by: Artem Gindinson <artem.gindinson@intel.com>

…4685) With `-sycldevice` triple environment removal (as performed in #3929), backwards compatibility has been enforced for SYCL objects/object-format libraries only. To ensure that `.a` libraries (archives) built by older compiler versions are usable with newer compiler version, adjust `clang-offload-deps` to consider `-sycldevice`-marked symbols when encountering a SYCL target. Signed-off-by: Artem Gindinson <artem.gindinson@intel.com>

Recent changes in the DPC++ compiler (intel/llvm#3929) allow to disregard the `sycldevice` component from the target triple in command lines. From there on, IR files for the Translator tests should be produced using the simplified target triple. Clean up the existing tests to match this change. Signed-off-by: Artem Gindinson <artem.gindinson@intel.com>

bader linked an issue Jun 12, 2021 that may be closed by this pull request

Can we drop target triple environment component -sycldevice? #3534

Closed

bader mentioned this pull request Jun 24, 2021

[Driver][SYCL] Allow for known aliases to be used for -fsycl-targets #3984

Merged

bader mentioned this pull request Aug 4, 2021

[SYCL] Correct some driver diagnostic wording #4243

Merged

bader added 12 commits August 5, 2021 12:52

[DAE] Remove unnecessary check for sycldevice environment

e2f918c

`CheckSpirKernels` flag is enabled only in SYCL mode, so there is no need in checking triple for that.

Remove SYCL specific SPIR target data information.

d8a5a67

It's not necessary.

[libdevice] Remove sycldevice environment component.

4c57b23

Updated documentation

fecbe1d

Drop sycldevice component from the design document example. Drop sycldevice component from the matrix extension examples.

Update test comments.

50b9ef5

[Driver] Replace sycldevice environment triple component with offlo…

464dad8

…ad kind check.

Support bundles with deprecated triple

0b0317a

Add a workaround to clang-offload-bundler to support bundles produced by previous versions of the compiler

[SYCL] Update sycl tests

aa2b05b

Remove sycldevice environment component.

83ef2de

Remove sycldevice environment triple component from llvm-spirv tests.

041c1bf

Remove sycldevice environment triple component from llvm tests.

14e0d63

Remove sycldevice environment triple component from clang tests.

46a928f

bader force-pushed the remove-sycldevice branch from 8184779 to 46a928f Compare August 5, 2021 14:55

bader changed the title ~~Remove sycldevice target triple environment component~~ [SYCL] Remove sycldevice target triple environment component Aug 5, 2021

mdtoguchi reviewed Aug 5, 2021

View reviewed changes

clang/include/clang/Basic/DiagnosticDriverKinds.td Outdated Show resolved Hide resolved

bader added 2 commits August 6, 2021 08:35

Improve deprecation diagnostics message.

fcde634

Merge remote-tracking branch 'intel/sycl' into remove-sycldevice

6940546

jinge90 previously approved these changes Aug 26, 2021

View reviewed changes

bader mentioned this pull request Aug 26, 2021

Fix test for bf16 conversions #4411

Merged

bader added 2 commits September 1, 2021 12:26

Merge remote-tracking branch 'intel/sycl' into remove-sycldevice

176d9bf

Remove new usages of -sycldevice triple compoenent.

f368708

bader dismissed stale reviews from jinge90, romanovvlad, sndmitriev, mdtoguchi, DenisBakhvalov, elizabethandrews, AaronBallman, and AlexeySachkov via f368708 September 1, 2021 12:34

bader requested a review from kychendev as a code owner September 1, 2021 12:34

mlychkov approved these changes Sep 1, 2021

View reviewed changes

AGindinson mentioned this pull request Sep 1, 2021

[Test] Drop 'sycldevice' from IR modules' triple information KhronosGroup/SPIRV-LLVM-Translator#1193

Merged

AGindinson approved these changes Sep 2, 2021

View reviewed changes

bader merged commit 29a1f16 into intel:sycl Sep 2, 2021

bader deleted the remove-sycldevice branch September 2, 2021 10:08

AGindinson mentioned this pull request Sep 2, 2021

[Driver] Properly set offload-deps target triple #4305

Merged

npmiller mentioned this pull request Sep 2, 2021

[SYCL][ROCM] amdgcn flag shows lld errors #4468

Closed

premanandrao mentioned this pull request Sep 10, 2021

[SYCL] Add support for __regcall calling convention to spir targets. #4533

Merged

AGindinson mentioned this pull request Oct 1, 2021

[SYCL] Fix backwards compatibility for libraries in archive format #4685

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SYCL] Remove sycldevice target triple environment component #3929

[SYCL] Remove sycldevice target triple environment component #3929

bader commented Jun 12, 2021 •

edited

Loading

bader commented Jun 12, 2021

bader commented Jun 12, 2021

bader commented Jun 12, 2021 •

edited

Loading

bader commented Jul 22, 2021

mdtoguchi commented Jul 22, 2021

bader commented Jul 22, 2021

bader commented Aug 5, 2021

bader commented Aug 5, 2021

vzakhari commented Aug 5, 2021

bader commented Aug 5, 2021

bader commented Sep 1, 2021

AGindinson left a comment

[SYCL] Remove sycldevice target triple environment component #3929

[SYCL] Remove sycldevice target triple environment component #3929

Conversation

bader commented Jun 12, 2021 • edited Loading

bader commented Jun 12, 2021

bader commented Jun 12, 2021

bader commented Jun 12, 2021 • edited Loading

bader commented Jul 22, 2021

mdtoguchi commented Jul 22, 2021

bader commented Jul 22, 2021

bader commented Aug 5, 2021

bader commented Aug 5, 2021

vzakhari commented Aug 5, 2021

bader commented Aug 5, 2021

bader commented Sep 1, 2021

AGindinson left a comment

Choose a reason for hiding this comment

bader commented Jun 12, 2021 •

edited

Loading

bader commented Jun 12, 2021 •

edited

Loading