Skip to content

[SYCL][Driver] Do not store AOT-specific options in the image descriptor #1428

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.

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

Merged

Conversation

AGindinson
Copy link
Contributor

@AGindinson AGindinson commented Mar 30, 2020

The OpenCL specification explicitly states that clProgramBuild must be
called after clProgramCreateWithBinary. Thus, AOT-compiler specific
options, if present in the image descriptor, may not be understood by
the RT implementation. This has shown up in FPGA Emulator execution,
and could potentially affect other targets.

Short-term: only store -compile-opts and -link-opts for SPIR-V
images, which will undergo JIT compilation.
Long-term: consider providing a mechanism for passing
specification-defined compile/link options separately from
offline compiler UI-specific ones.

Signed-off-by: Artem Gindinson [email protected]

The OpenCL specification explicitly states that clProgramBuild must be
called after clProgramCreateWithBinary. Thus, AOT-compiler specific
options, if present in the image descriptor, may not be understood by
the RT implementation. This has shown up in FPGA Emulator execution,
and could potentially affect other targets.

Short-term: only store -compile-opts and -link-opts for SPIR-V
images, which will undergo JIT compilation.
Long-term: consider providing a mechanism for passing
specification-defined compile/link options separately from
offline compiler UI-specific ones.

Signed-off-by: Artem Gindinson <[email protected]>
@AlexeySachkov
Copy link
Contributor

The OpenCL specification explicitly states that clProgramBuild can be called after clProgramCreateWithBinary.

must be called

@AGindinson AGindinson force-pushed the private/agindins/aot-compile-opts branch from 9e69992 to 36707be Compare March 30, 2020 12:09
@AGindinson
Copy link
Contributor Author

must be called

Thanks @AlexeySachkov, I've updated the commit message.

Copy link
Contributor

@mdtoguchi mdtoguchi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@romanovvlad romanovvlad merged commit f6d6baa into intel:sycl Mar 30, 2020
@AGindinson AGindinson deleted the private/agindins/aot-compile-opts branch March 30, 2020 15:48
alexbatashev pushed a commit to alexbatashev/llvm that referenced this pull request Apr 15, 2020
…c_abi_checks

* origin/sycl: (6966 commits)
  [NFC][SYCL] Do not add `sycl_device` attribute to OpenCL kernel (intel#1439)
  [SYCL][NFC] Move SYCL pipe metadata call to be inside the null check for D (intel#1436)
  [SYCL][NFC] Move template function definition to .h file (intel#1433)
  [SYCL] Don't expose vector of booleans as storage format (intel#1419)
  [SYCL] Don't throw exceptions from destructors (intel#1378)
  [BuildBot] Add support for multiple CMake options (intel#1434)
  [SYCL][NFC] Fix warning inline namespace reopened as no-inline (intel#1435)
  [SYCL] Check if loadPlugin returns a nullptr (intel#1411)
  [SYCL] Release notes for February'20 SYCL implementation update (intel#1400)
  [SYCL][Doc] Improve contribution guidelines (intel#1422)
  [BuildBot] Add --cmake-opts option to configure.py script (intel#1430)
  [SYCL] Enable non-read-write memory object mapping in scheduler (intel#1390)
  [SYCL][Driver] Do not store AOT-specific options in the image descriptor (intel#1428)
  [Driver][NFC] Fix string problem used for tracking duplicate triples (intel#1424)
  [SYCL][NFC] Use the non-deprecated setAlignment() in LowerWGScope (intel#1420)
  [SYCL][NFC] Fix formatting in GetStartedGuide (intel#1417)
  [NFC] Move CODEOWNERS file to enable GitHub automation (intel#1418)
  [SYCL] Add test for private array init by zeroes (intel#1402)
  [Driver][SYCL][FPGA] Adjust device and AOCX link order for FPGA (intel#1389)
  [SYCL] Change runtime check to assert in program_manager.cpp (intel#1413)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants