Skip to content

[SYCL] Fix warnings on clang-based build #3570

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 6 commits into from
Apr 19, 2021
Merged

Conversation

pvchupin
Copy link
Contributor

@pvchupin pvchupin commented Apr 17, 2021

  • CompilerInvocation.cpp: error: '&&' within '||'
    [-Werror,-Wlogical-op-parentheses]
  • clang/lib/Sema/SemaSYCL.cpp: error: private field 'Footer' is not used
    [-Werror,-Wunused-private-field]
  • os_util.cpp: comparison of integers of different signs: 'int' and
    'const unsigned long long' [-Werror,-Wsign-compare]

Signed-off-by: Pavel V Chupin [email protected]

@pvchupin pvchupin requested a review from vladimirlaz April 17, 2021 08:26
@pvchupin pvchupin changed the title Fix warnings on Windows build [SYCL] Fix warnings on Windows build Apr 17, 2021
* CompilerInvocation.cpp: error: '&&' within '||'
  [-Werror,-Wlogical-op-parentheses]
* clang/lib/Sema/SemaSYCL.cpp: error: private field 'Footer' is not used
  [-Werror,-Wunused-private-field]
* SPIRVUtil.cpp: error: private field 'RetTy' is not used
  [-Werror,-Wunused-private-field]
* os_util.cpp: comparison of integers of different signs: 'int' and
  'const unsigned long long' [-Werror,-Wsign-compare]

Signed-off-by: Pavel V Chupin <[email protected]>
@@ -1720,6 +1720,9 @@ class OpenCLStdToSPIRVFriendlyIRMangleInfo : public BuiltinFuncMangleInfo {
private:
OCLExtOpKind ExtOpId;
ArrayRef<Type *> ArgTys;
#ifdef __clang__
[[maybe_unused]]
#endif
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@AlexeySotkin, @AlexeySachkov, can you take care of this please?

Copy link
Contributor Author

@pvchupin pvchupin Apr 18, 2021

Choose a reason for hiding this comment

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

@pvchupin pvchupin changed the title [SYCL] Fix warnings on Windows build [SYCL] Fix warnings on clang-based build Apr 17, 2021
Copy link
Contributor

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

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

We should not be testing for the compiler like that before using an attribute -- it's not a portable fix for the issue. Instead, use the helper macros from Compiler.h.

Pavel Chupin and others added 4 commits April 17, 2021 11:05
@pvchupin
Copy link
Contributor Author

We should not be testing for the compiler like that before using an attribute -- it's not a portable fix for the issue. Instead, use the helper macros from Compiler.h.

It's appeared that LLVM_ATTRIBUTE_UNUSED is not portable as well. Producing warning/error with G++.

Copy link
Contributor

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

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

The better fix for the unused field was to change Compiler.h so that the macro works more portably, but because this is only a temporary measure while we implement the integration footer, the new approach is good enough for me.

LG!

Copy link
Contributor

@premanandrao premanandrao left a comment

Choose a reason for hiding this comment

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

LGTM

@premanandrao
Copy link
Contributor

Please fix the description. SPIRVUtil.cpp changes are no longer part of this PR.

@againull againull merged commit 3977679 into intel:sycl Apr 19, 2021
@pvchupin pvchupin deleted the fix-warnings branch April 19, 2021 19:28
alexbatashev pushed a commit to alexbatashev/llvm that referenced this pull request Apr 20, 2021
* sycl: (625 commits)
  [SYCL] Fix post-commit build failure (intel#3578)
  [SYCL] Add support for set(get)_specialization_constant (intel#3501)
  [SYCL] Do not allow template instantiation to create null attributes. (intel#3575)
  [SYCL][PI][L0] Close and submit batch immediately when Queue is empty. (intel#3552)
  [SYCL] Raise bit_cast to SYCL namespace (intel#3524)
  [ESIMD] Always preserve -vc-codegen option for ESIMD kernels (intel#3547)
  [SYCL] Fix warnings on clang-based build (intel#3570)
  Revert "Align tests with the codegen changes"
  Disable SPV_INTEL_memory_access_aliasing extension
  Fix build issues after applying translator patches
  [PassManager][PhaseOrdering] lower expects before running simplifyCFG
  Exclude spirv.hpp for clang-format
  Fix llvm-spirv crash when count of Fortran metadata variables is an array
  Fix transTypeComposite bug (intel#964)
  Fix incorrect translation of FPGA decoration on arrays (intel#983)
  Tolerate more inputs during alias.scope/noalias MD translation (intel#994)
  Update for LLVM iterator change
  Fix crash at translation of Entity of DebugImportedEntity (intel#951)
  Add HandleLLVMOptions to main CMakeLists.txt to propagate configuration flags. In particular, this enables Multi-threaded option in MSVC (instead of Multi-threaded DLL)
  Fix uninitialized variables warnings These warnings come into place when HandleLLVMOptions is imported
  ...
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