Skip to content

[SPIR-V] Enabling SPIR-V builtin lookup in device SYCL mode#1384

Merged
bader merged 5 commits intointel:syclfrom
codeplaysoftware:victor/spirv-builtin-no-atomics
Mar 26, 2020
Merged

[SPIR-V] Enabling SPIR-V builtin lookup in device SYCL mode#1384
bader merged 5 commits intointel:syclfrom
codeplaysoftware:victor/spirv-builtin-no-atomics

Conversation

@Naghasan
Copy link

@Naghasan Naghasan commented Mar 24, 2020 •
edited by bader
Loading

This patch enables implicitly fdeclare-spirv-builtins when building sycl code in device mode and the runtime header changes related to flag being enabled. It also reverts some of the manual mangling done in libclc.

Signed-off-by: Victor Lomuller victor@codeplay.com

@Naghasan Naghasan force-pushed the victor/spirv-builtin-no-atomics branch 2 times, most recently from 8f72079 to 8fda3cd Compare March 24, 2020 13:40
bader
bader reviewed Mar 24, 2020
View reviewed changes
@bader
Copy link
Contributor

bader commented Mar 24, 2020

@Naghasan, could you resolve the conflicts, please?

@Naghasan Naghasan force-pushed the victor/spirv-builtin-no-atomics branch from 8fda3cd to c188011 Compare March 24, 2020 16:39
@Naghasan
Copy link
Author

Naghasan commented Mar 24, 2020

@Naghasan, could you resolve the conflicts, please?

Done

@Naghasan Naghasan force-pushed the victor/spirv-builtin-no-atomics branch from c188011 to 5312701 Compare March 24, 2020 17:54
@Naghasan Naghasan changed the title [WIP][Do not Merge][SPIR-V] Enabling SPIR-V builtin lookup in device SYCL mode [SPIR-V] Enabling SPIR-V builtin lookup in device SYCL mode Mar 24, 2020
@bader bader added libclc libclc project related issues cuda CUDA back-end labels Mar 25, 2020
bader
bader previously approved these changes Mar 25, 2020
View reviewed changes
@bader bader requested a review from mdtoguchi March 25, 2020 06:31
@bader
Copy link
Contributor

bader commented Mar 25, 2020 •
edited
Loading

@Naghasan, any ideas why macro is not defined?
From http://ci.llvm.intel.com:8010/#/builders/28/builds/1203/steps/16/logs/FAIL__SYCL__aliases_cpp:

d:\buildbot\product_worker_intel\build_pr_with_lit_win\llvm.obj\bin\..\include\sycl\CL/__spirv/spirv_ops.hpp:25:5: error: invalid token at start of a preprocessor expression
#if not defined(__SPIRV_BUILTIN_DECLARATIONS__)
    ^
1 error generated.
error: command failed with exit status: 1

Victor Lomuller added 4 commits March 25, 2020 09:44
[SYCL] Enable -fdeclare-spirv-builtins for SYCL device compilation mode
a8482b1 
For device compilation, SPIR-V builtins are now looked up by
the device compiler. They now longer need to be forward declared.

Signed-off-by: Victor Lomuller <victor@codeplay.com>
[SYCL-PTX] Revert manual mangling of some SPIR-V builtins
e02e118 
Signed-off-by: Victor Lomuller <victor@codeplay.com>
[SYCL-PTX] Add fmod builtin
4b0b0a9 
Signed-off-by: Victor Lomuller <victor@codeplay.com>
[SYCL-PTX] Update Atomic mangling
377b2e3 
Signed-off-by: Victor Lomuller <victor@codeplay.com>
@Naghasan Naghasan force-pushed the victor/spirv-builtin-no-atomics branch from 5312701 to 377b2e3 Compare March 25, 2020 09:45
@Naghasan
Copy link
Author

Naghasan commented Mar 25, 2020

any ideas why macro is not defined?

The macro is defined, the #if not is not portable. I changed it to a #ifndef

bader
bader previously approved these changes Mar 25, 2020
View reviewed changes
@bader
Copy link
Contributor

bader commented Mar 25, 2020

any ideas why macro is not defined?

The macro is defined, the #if not is not portable. I changed it to a #ifndef

This seems to work, but there are still 10 failed LIT tests on Windows.

[SPIR-V] Change long definition to deal with LLP64 vs LP64
72c40d7 
Signed-off-by: Victor Lomuller <victor@codeplay.com>
@Naghasan
Copy link
Author

Naghasan commented Mar 25, 2020

any ideas why macro is not defined?

The macro is defined, the #if not is not portable. I changed it to a #ifndef

This seems to work, but there are still 10 failed LIT tests on Windows.

I neglected LLP64 vs LP64 ... I made a fix for this but limits inputs to be int<len>_t for now. As there is a systematic type check/conversion before calling the builtins that should be OK for now. But it is not perfect though, I'll work a follow up and more flexible solution.

mdtoguchi
mdtoguchi reviewed Mar 25, 2020
View reviewed changes
@keryell
Copy link
Contributor

keryell commented Mar 25, 2020

From http://ci.llvm.intel.com:8010/#/builders/28/builds/1203/steps/16/logs/FAIL__SYCL__aliases_cpp:

d:\buildbot\product_worker_intel\build_pr_with_lit_win\llvm.obj\bin\..\include\sycl\CL/__spirv/spirv_ops.hpp

This mix and match of \ and / is a little bit scary at the first place...
But not related to this issue.

@bader bader merged commit 64b771a into intel:sycl Mar 26, 2020
@Naghasan Naghasan deleted the victor/spirv-builtin-no-atomics branch March 26, 2020 09:17
This was referenced Mar 26, 2020
Merged
Merged
alexbatashev added a commit to alexbatashev/llvm that referenced this pull request Mar 27, 2020
@alexbatashev
Merge remote-tracking branch 'upstream/sycl' into private/abatashe/sp…
6790032 
…hinx

* upstream/sycl: (357 commits)
  [Support] Implement a simple tabular data management library (intel#1358)
  [Support] Implement a property set I/O library (intel#1357)
  [SYCL] Fix buffer constructor using iterators (intel#1386)
  [SYCL][FPGA] Enable a set of loop attributes (intel#1312)
  [Driver][SYCL][FPGA] Proper dependency output location when given /Fo<dir> (intel#1346)
  [SPIR-V] Enabling SPIR-V builtin lookup in device SYCL mode (intel#1384)
  [SYCL][NFC] Unify setting kernel arguments (intel#1379)
  [SYCL][Doc] First revision of standard layout relaxation extension (intel#1344)
  [SYCL] Fixed sub-buffer alloca search (intel#1385)
  [SYCL][FPGA] Emit multiple IR variants for the IVDep attribute (intel#1383)
  [SYCL] Add experimental flag to enable front-end optimizations (intel#1376)
  [SYCL] Remove unexpected double in complex SPIR-V for float support (intel#1381)
  [SYCL] Default work-group sizes based on max (intel#952)
  [SYCL][CUDA] Fix usage of multiple backends in the same program (intel#1252)
  [SPIR-V] Add SPIR-V builtin definitions to the builtin lookup.
  [SPIR-V] Add macro definition when -fdeclare-spirv-builtins is activated
  [SYCL] Fix sycl_generic printing
  [SYCL] Support intel::reqd_work_group_size (intel#1328)
  [SYCL][NFC] Make the RT::PiPlugin object private (intel#1375)
  [SPIRV] Add convergent attribute to SPIR-V built-ins (intel#1373)
  ...
@Fznamznon Fznamznon mentioned this pull request Mar 30, 2020
Open
@vzakhari vzakhari mentioned this pull request Mar 31, 2020
Closed
@AlexeySachkov AlexeySachkov mentioned this pull request Apr 1, 2020
Merged
@Naghasan Naghasan mentioned this pull request Apr 2, 2020
Merged
bader pushed a commit that referenced this pull request Apr 6, 2020
@AlexeySachkov
[NFC] Cleanup unneded macro from builtins implementation (#1445)
3238b3f 
Removed macro which are useless since #1384

Signed-off-by: Alexey Sachkov <alexey.sachkov@intel.com>
@bader bader mentioned this pull request Jun 13, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bader bader bader approved these changes

@mdtoguchi mdtoguchi mdtoguchi left review comments

Assignees

No one assigned

Labels

cuda CUDA back-end libclc libclc project related issues

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Naghasan @bader @keryell @mdtoguchi