Skip to content
This repository has been archived by the owner on Mar 21, 2024. It is now read-only.

Enable more warning flags. #1359

Merged
merged 22 commits into from
Feb 16, 2021

Conversation

alliepiper
Copy link
Collaborator

@alliepiper alliepiper commented Dec 11, 2020

Fixes NVIDIA/cub#228.
Fixes #1273.

Requires NVIDIA/cub#249.

@alliepiper alliepiper marked this pull request as draft December 11, 2020 02:45
@alliepiper alliepiper added this to the 1.12.0 milestone Dec 11, 2020
@alliepiper
Copy link
Collaborator Author

run tests

@alliepiper
Copy link
Collaborator Author

run tests

cmake/ThrustBuildCompilerTargets.cmake Outdated Show resolved Hide resolved
cmake/ThrustBuildCompilerTargets.cmake Show resolved Hide resolved
cmake/ThrustBuildCompilerTargets.cmake Show resolved Hide resolved
cmake/ThrustBuildCompilerTargets.cmake Outdated Show resolved Hide resolved
cmake/ThrustBuildCompilerTargets.cmake Outdated Show resolved Hide resolved
testing/functional_placeholders_bitwise.cu Show resolved Hide resolved
testing/set_symmetric_difference.cu Show resolved Hide resolved
thrust/system/cuda/detail/future.inl Show resolved Hide resolved
thrust/system/detail/generic/sequence.inl Show resolved Hide resolved
const uint64_t key) const {
uint64_t hash0 = thrust::random::taus88(value)();
const uint64_t key_) const {
uint64_t hash0 = thrust::random::taus88(static_cast<uint32_t>(value))();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can this be std::uint32_t and std::uint64_t?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No -- tau88 only accepts uint32_t inputs. This doesn't change anything, it just makes the implicit cast explicit and silences the conversion warnings.

I checked with the shuffle author about this change and he was fine with it, it shouldn't affect the results.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I just meant stick std:: in front of the types.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

@alliepiper
Copy link
Collaborator Author

run tests

@alliepiper alliepiper force-pushed the enh/pedantic_flags/gh.cub228 branch 2 times, most recently from 1a0ff3a to 46db957 Compare January 23, 2021 02:55
@alliepiper
Copy link
Collaborator Author

run tests

@alliepiper
Copy link
Collaborator Author

run tests

@alliepiper
Copy link
Collaborator Author

run tests

@alliepiper
Copy link
Collaborator Author

run tests

@alliepiper
Copy link
Collaborator Author

DVS CL: 29547313

@alliepiper alliepiper marked this pull request as ready for review January 26, 2021 21:00
@alliepiper alliepiper added testing: internal ci in progress Currently testing on internal NVIDIA CI (DVS). testing: gpuCI passed Passed gpuCI testing. labels Jan 26, 2021
@alliepiper alliepiper linked an issue Jan 26, 2021 that may be closed by this pull request
@alliepiper alliepiper added testing: internal ci passed Passed internal NVIDIA CI (DVS). testing: gpuCI passed Passed gpuCI testing. and removed testing: gpuCI in progress Started gpuCI testing. labels Feb 10, 2021
Replaces the macros for:
- THRUST_CONSTEXPR
- THRUST_OVERRIDE
- THRUST_DEFAULT
- THRUST_NOEXCEPT
- THRUST_FINAL
MSVC /W4 issues warnings when this could be used but isn't.

We still have to suppress the MSVC warnings since there's no way to
silence them pre-C++17.
Anonymous structs are C features. In C++, they're non-portable
compiler extensions.

These only seemed to pop up in CUB-style `TempStorage` objects, I just
picked some reasonable sounding names for them.
Some tests throw exceptions unconditionally and this warning is safe
to ignore in such cases.

Added a `thrust.silence_unreachable_code_warnings` interface target to
collect various compiler flags that disable these warnings. This can be
linked selectively in a per-test `<testname>.cmake` file to only disable
warnings on the tests where this behavior is expected.
The unittest_static_assert test needs to be disable for TBB/OMP hosts,
too.
The `cuda_std_17` compile feature is broken for MSVC when
CMake < 3.18.3.
The expression `(n + d - 1) / d` can overflow the numerator. The
new method avoids that.

See NVIDIA/cub#221 for reference.
Includes a workaround that fixes NVIDIA#1273.
@alliepiper alliepiper merged commit df8afb8 into NVIDIA:main Feb 16, 2021
@alliepiper alliepiper deleted the enh/pedantic_flags/gh.cub228 branch February 16, 2021 18:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
testing: gpuCI passed Passed gpuCI testing. testing: internal ci passed Passed internal NVIDIA CI (DVS).
Projects
None yet
2 participants