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.

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

[SYCL] Add buffer dimensions restriction #1147

Merged
merged 6 commits into from
Mar 10, 2020

Conversation

MochalovaAn
Copy link
Contributor

@MochalovaAn MochalovaAn commented Feb 19, 2020

According to the specification, a buffer can have dimensions of only 1, 2, and 3. In the old version, "static_assert" was used to limit it, which did not throw an exception when creating a pointer to a buffer with an incorrect dimension. In this patch it is impossible to create not only an object of the buffer class with incorrect dimensions, but also a link to it.
Also, constructor calls in the accessos class that tried to instantiate a buffer with a zero dimension were fixed.

Signed-off-by: amochalo anastasiya.mochalova@intel.com

@bader
Copy link
Contributor

bader commented Feb 19, 2020

Please, apply clang-format.

@@ -1250,7 +1251,7 @@ class accessor<DataT, Dimensions, AccessMode, access::target::image_array,
};

} // namespace sycl
} // __SYCL_INLINE_NAMESPACE(cl)
} // namespace cl
Copy link
Contributor

Choose a reason for hiding this comment

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

Please revert this change

Comment on lines 776 to 779
template <int Dims = Dimensions, typename AllocatorT,
typename = typename detail::enable_if_t<
(Dims == 0) &&
(!IsPlaceH && (IsGlobalBuf || IsConstantBuf))>
>
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this code properly clang-formatted?

-Replacing "static_assetr" with "enable_if" for correct work with pointers
-fixing some functions for correct "make sycl-check"

Signed-off-by: amochalo <anastasiya.mochalova@intel.com>
Signed-off-by: amochalo <anastasiya.mochalova@intel.com>
Signed-off-by: amochalo <anastasiya.mochalova@intel.com>
Signed-off-by: amochalo <anastasiya.mochalova@intel.com>
@sergey-semenov
Copy link
Contributor

Please, apply clang-format to the patch

Signed-off-by: amochalo <anastasiya.mochalova@intel.com>
@sergey-semenov
Copy link
Contributor

The contents of the patch LGTM, but could you please fix the commit message? It seems that the description refers to the diff between this patch and its earlier version.

@bader bader requested a review from sergey-semenov March 2, 2020 21:05
Signed-off-by: amochalo <anastasiya.mochalova@intel.com>
Copy link
Contributor

@sergey-semenov sergey-semenov left a comment

Choose a reason for hiding this comment

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

-fixing some functions for correct "make sycl-check"

Minor: this part of the commit message is not very informative, you should point out the specific problem these changes solve.

@bader bader merged commit fc03fda into intel:sycl Mar 10, 2020
alexbatashev added a commit to alexbatashev/llvm that referenced this pull request Mar 13, 2020
…st_commit

* otcshare/sycl: (469 commits)
  [SYCL] Implement thread-local storage restriction (intel#1281)
  [Driver][SYCL][FPGA] Adjust the output location for the project report (intel#1278)
  [SYCL][NFC] Fix static code analysis concerns (intel#1283)
  [SYCL] Fix the test/basic_tests/buffer/subbuffer.cpp (intel#1277)
  [SYCL][CUDA] Implement the program kernel names query (intel#1248)
  [SYCL] Honor the LLVM_LIBDIR_SUFFIX variable at installation time (intel#1261)
  [SYCL][UX] Diagnostic for undefined device functions (intel#1026)
  [SYCL] Reverse reqd_work_group_size attribute (intel#1234)
  [SYCL] Rename project to oneAPI DPC++ Compiler (intel#1249)
  [SYCL][XPTI] Instrumentation of SYCL runtime with XPTI (intel#1129)
  [SYCL] Add buffer dimensions restriction (intel#1147)
  [SYCL][NFC] Update copyright header in handler files (intel#1271)
  [SYCL][NFC] Format the code with clang-format
  [SYCL][Test] Fix SYCL library location path for LIT tests (intel#1228)
  [SYCL][NFC] Fix doxygen warnings (intel#1270)
  [SYCL][CUDA] Add the CUDA backend to the deploy-sycl-toolchain target (intel#1268)
  [SYCL][NFC] Fix a misleading comment regarding the SYCL flow (intel#1266)
  Change capability for SpecId decoration
  README.md: Mention retrieving llvm archive signatures
  travis: Restore macOS builds
  ...
alexbatashev pushed a commit to alexbatashev/llvm that referenced this pull request Mar 15, 2020
…_accessor_refactor

* origin/sycl: (454 commits)
  [SYCL][NFC] Fix static code analysis concerns (intel#1283)
  [SYCL] Fix the test/basic_tests/buffer/subbuffer.cpp (intel#1277)
  [SYCL][CUDA] Implement the program kernel names query (intel#1248)
  [SYCL] Honor the LLVM_LIBDIR_SUFFIX variable at installation time (intel#1261)
  [SYCL][UX] Diagnostic for undefined device functions (intel#1026)
  [SYCL] Reverse reqd_work_group_size attribute (intel#1234)
  [SYCL] Rename project to oneAPI DPC++ Compiler (intel#1249)
  [SYCL][XPTI] Instrumentation of SYCL runtime with XPTI (intel#1129)
  [SYCL] Add buffer dimensions restriction (intel#1147)
  [SYCL][NFC] Update copyright header in handler files (intel#1271)
  [SYCL][NFC] Format the code with clang-format
  [SYCL][Test] Fix SYCL library location path for LIT tests (intel#1228)
  [SYCL][NFC] Fix doxygen warnings (intel#1270)
  [SYCL][CUDA] Add the CUDA backend to the deploy-sycl-toolchain target (intel#1268)
  Change capability for SpecId decoration
  README.md: Mention retrieving llvm archive signatures
  travis: Restore macOS builds
  Fix DebugInfo creation after LLVM change 7a42bab
  Add more missing mixes
  Add missing fixes
  ...
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.

5 participants