Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 5 commits into from
Mar 26, 2020
Merged

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

merged 5 commits into from
Mar 26, 2020

Conversation

Naghasan
Copy link
Contributor

@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
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
Contributor Author

@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

Naghasan added 4 commits March 25, 2020 09:44
@Naghasan
[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>
@Naghasan
[SYCL-PTX] Revert manual mangling of some SPIR-V builtins
e02e118 
Signed-off-by: Victor Lomuller <victor@codeplay.com>
@Naghasan
[SYCL-PTX] Add fmod builtin
4b0b0a9 
Signed-off-by: Victor Lomuller <victor@codeplay.com>
@Naghasan
[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
Contributor Author

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.

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

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.

@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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mdtoguchi mdtoguchi mdtoguchi left review comments

@bader bader bader approved these changes

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