Skip to content

[NFC] [SYCL] Make code a bit cleaner when using DISABLE_SYCL_INSTRUMENTATION_METADATA #3401

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
merged 9 commits into from
Mar 26, 2021

Conversation

s-kanaev
Copy link
Contributor

No description provided.

@s-kanaev s-kanaev requested a review from a team as a code owner March 23, 2021 12:30
bader
bader previously requested changes Mar 23, 2021
Sergey Kanaev added 2 commits March 23, 2021 15:35
Signed-off-by: Sergey Kanaev <[email protected]>
Signed-off-by: Sergey Kanaev <[email protected]>
@bader bader dismissed their stale review March 23, 2021 12:45

Please, apply clang-format changes to fix CI check.

Sergey Kanaev added 2 commits March 23, 2021 15:47
Signed-off-by: Sergey Kanaev <[email protected]>
Signed-off-by: Sergey Kanaev <[email protected]>
@s-kanaev
Copy link
Contributor Author

Please, apply clang-format changes to fix CI check.

Done.

@s-kanaev s-kanaev requested a review from bader March 24, 2021 08:32
tovinkere
tovinkere previously approved these changes Mar 24, 2021
Copy link
Contributor

@tovinkere tovinkere left a comment

Choose a reason for hiding this comment

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

Suggestion: If we are going to turn it off by default and manage enabling through compiler command-line define to ensure it is an opt-in capability, it would make sense to reverse the logic and the macro name to ENABLE_SYCL_INSTRUMENTATION_METADATA. If this can be hidden by the compiler front end, then I suppose it shouldn't matter.

LGTM otherwise.

@s-kanaev
Copy link
Contributor Author

s-kanaev commented Mar 25, 2021

Suggestion: If we are going to turn it off by default and manage enabling through compiler command-line define to ensure it is an opt-in capability, it would make sense to reverse the logic and the macro name to ENABLE_SYCL_INSTRUMENTATION_METADATA. If this can be hidden by the compiler front end, then I suppose it shouldn't matter.

I think it should be done as distinct patch

Signed-off-by: Sergey Kanaev <[email protected]>
@bader bader merged commit 895b4f1 into intel:sycl Mar 26, 2021
againull pushed a commit that referenced this pull request Jun 2, 2021
alexbatashev pushed a commit to alexbatashev/llvm that referenced this pull request Jun 4, 2021
* sycl: (320 commits)
  [SYCL] Silence a "local variable is initialized but not referenced" warning; NFC (intel#3870)
  [SYCL] Improve SYCL_DEVICE_ALLOWLIST (intel#3826)
  [SPIR-V] Change return value of mapType function (intel#3871)
  [SYCL] Fix post-commit failure in handler.hpp from unused-parameters. (intel#3874)
  [Driver][SYCL] Do not imply defaultlib msvcrt for Linux based driver on Windows (intel#3827)
  [SYCL] Unique stable name rebase (intel#3835)
  [SYCL] Align behavior of empty command groups with SYCL2020 (intel#3822)
  [SYCL][ESIMD] Make typenames and constants consistent with SYCL API style. (intel#3850)
  [SYCL] Allow __failed_assertion to support libstdc++-11 (intel#3774)
  [SYCL] Refactor stream class handing implementation (intel#3646)
  [SYCL] Fix syntax error introduced in intel#3401 (intel#3861)
  [SYCL] SYCL 2020 sub_group algorithms (intel#3786)
  [Buildbot][NFC] Add option to use LLD as linker (intel#3866)
  Revert "Emit correct location lists with basic block sections."
  [SPIRITTAnnotations] Fix debug info for ITT calls. (intel#3829)
  [SYCL][Doc] Fix build of Sphinx docs (intel#3863)
  [SYCL][FPGA][NFC] Tidy up intel_fpga_reg codegen test (intel#3810)
  [CODEOWNERS] Fix SPIRITTAnnnotations tests ownership (intel#3859)
  [SYCL][ESIMD] Host-compile simd.cpp test, fix errors & warnings. (intel#3846)
  [SYCL] Store pointers to memory allocations instead of iterators (intel#3860)
  ...
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