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

Add .clang-tidy and fix clang-tidy warnings #857

Merged
merged 78 commits into from
Sep 16, 2021
Merged
Show file tree
Hide file tree
Changes from 58 commits
Commits
Show all changes
78 commits
Select commit Hold shift + click to select a range
debe302
Clean up: use std::size_t, include cstddef and aligned.hpp where missing
harrism Aug 25, 2021
86e7859
Fix copyright.
harrism Aug 25, 2021
b9f7b42
More missed std::size_t
harrism Aug 30, 2021
d7f1a32
doc
harrism Aug 30, 2021
8d3a311
Merge branch 'branch-21.10' into fea-std-size_t-and-includes
harrism Aug 30, 2021
97f5571
.clang-tidy and initial fixes
harrism Aug 31, 2021
39f067e
Merge branch 'branch-21.10' into fea-clang-tidy
harrism Aug 31, 2021
d9b9ab4
Suppress cppcoreguidelines-macro-usage
harrism Aug 31, 2021
f65249b
parameter name
harrism Aug 31, 2021
118593a
Merge branch 'branch-21.10' into fea-clang-tidy
harrism Sep 1, 2021
401f2ae
tidying
harrism Sep 2, 2021
573dd36
tidy some tests
harrism Sep 2, 2021
be95503
tidy cuda_stream.hpp
harrism Sep 2, 2021
7c2653d
Remove incorrect fix for warning
harrism Sep 6, 2021
d108e80
include order
harrism Sep 6, 2021
d4ee0d4
tidying
harrism Sep 6, 2021
7aafe38
Use temporary fork of gtest
harrism Sep 7, 2021
b7b4300
function-cognitive-complexity.IgnoreMacros=1
harrism Sep 7, 2021
6143ffa
Tidy cuda_async_mr_tests
harrism Sep 7, 2021
2419b1c
Factor out byte_literals for reuse
harrism Sep 7, 2021
507176a
tidy limiting_mr_tests
harrism Sep 7, 2021
3820f1b
tidy logger_tests
harrism Sep 7, 2021
2580056
tidy mr_tests
harrism Sep 7, 2021
633261a
Fix device_scalar_test hang
harrism Sep 7, 2021
0390808
Fix arena.hpp debug build
harrism Sep 7, 2021
973bc99
tidy device_scalar
harrism Sep 7, 2021
c2402b6
suppress pointer arith warnings
harrism Sep 7, 2021
27fbde4
tidy device_buffer
harrism Sep 7, 2021
c476742
Suppress owning-memory warnings
harrism Sep 7, 2021
a1162b1
NOLINT macro parentheses
harrism Sep 7, 2021
fb46f80
Merge branch 'branch-21.10' into fea-clang-tidy
harrism Sep 7, 2021
77fdc94
tidy free lists
harrism Sep 7, 2021
c54c513
tidy stream_ordered_memory_resource and free_lists
harrism Sep 7, 2021
a4e0d9a
tidy device_memory_resource and aligned_resource_adaptor
harrism Sep 7, 2021
a6a0cab
Fix nodiscard compilation error
harrism Sep 8, 2021
a12bb90
tidying more MRs
harrism Sep 8, 2021
2b1b49d
Remove `alignment_type` and ignore swappable parameters when one is c…
harrism Sep 8, 2021
b149037
tidy arena_memory_resource
harrism Sep 8, 2021
2d410e3
tidy binning_memory_resource
harrism Sep 8, 2021
1811fe3
tidy fixed_size_mr
harrism Sep 8, 2021
d236810
tidy limiting_resource_adaptor
harrism Sep 8, 2021
5c21ec4
tidy logging_resource_adaptr
harrism Sep 8, 2021
afc9e76
tidy managed_mr
harrism Sep 8, 2021
86fcffd
tidy owning_wrapper and per_device_resource
harrism Sep 8, 2021
b808afd
tidy polymorphic_allocator
harrism Sep 8, 2021
9543e3f
tidy pool_mr
harrism Sep 8, 2021
e8e1eae
tidy statistics_resource_adaptor
harrism Sep 8, 2021
b26b9aa
tidy thread_safe_resource_adaptor
harrism Sep 8, 2021
7fb12d2
tidy thrust_allocator_adaptor
harrism Sep 8, 2021
2c28eee
tidy tracking_resource_adaptor
harrism Sep 8, 2021
c0c71a9
tidy host_mrs
harrism Sep 8, 2021
111a2f4
tidy aligned_mr_tests
harrism Sep 8, 2021
70b85c2
tidy device MR tests
harrism Sep 8, 2021
7ac76e6
tidy host mr tests
harrism Sep 8, 2021
9dcbae6
copyright
harrism Sep 8, 2021
1cfa493
tidy benchmarks and more
harrism Sep 8, 2021
41790ff
p->ptr
harrism Sep 8, 2021
514a4f1
Revert threshold size type
harrism Sep 8, 2021
d3d2b08
Disable readability-named-parameter
harrism Sep 8, 2021
a859235
Use C++17 for all of Cython
harrism Sep 9, 2021
0ac66e1
Avoid bounds check exception in `stack_trace`
harrism Sep 9, 2021
fe1d70d
variable name
harrism Sep 9, 2021
fbf1c9c
Revert magic numbers
harrism Sep 9, 2021
6f760b1
Merge branch 'branch-21.10' into fea-clang-tidy
harrism Sep 14, 2021
8d743a8
Fix merge problems
harrism Sep 14, 2021
397962b
More tidy warnings found on command line
harrism Sep 14, 2021
87959ec
Fix potential leak and exception warnings
harrism Sep 15, 2021
cca3880
Fix uninitialized member clang-tidy warning
harrism Sep 15, 2021
496474f
nodiscard
harrism Sep 15, 2021
6f4d739
tidying
harrism Sep 15, 2021
8459c72
nodiscard
harrism Sep 15, 2021
5b91598
nolints
harrism Sep 15, 2021
3bfaa55
nodiscard
harrism Sep 15, 2021
b7ea38c
nodiscard and nolint
harrism Sep 15, 2021
098e08a
cmake style
harrism Sep 15, 2021
c1f9872
cmake docstring
harrism Sep 15, 2021
20528cb
Merge branch 'branch-21.10' into fea-clang-tidy
harrism Sep 15, 2021
ce3b3ed
Fix device_uvector::set_element_async for non-fundamental types.
harrism Sep 15, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 61 additions & 0 deletions .clang-tidy
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
---
Checks: 'clang-diagnostic-*,
clang-analyzer-*,
cppcoreguidelines-*,
modernize-*,
bugprone-*,
performance-*,
readability-*,
llvm-*,
-modernize-use-trailing-return-type,
-cppcoreguidelines-macro-usage'
WarningsAsErrors: ''
HeaderFilterRegex: ''
AnalyzeTemporaryDtors: false
FormatStyle: none
CheckOptions:
- key: cert-dcl16-c.NewSuffixes
value: 'L;LL;LU;LLU'
- key: cert-oop54-cpp.WarnOnlyIfThisHasSuspiciousField
value: '0'
- key: cert-str34-c.DiagnoseSignedUnsignedCharComparisons
value: '0'
- key: cppcoreguidelines-explicit-virtual-functions.IgnoreDestructors
value: '1'
- key: cppcoreguidelines-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic
value: '1'
- key: google-readability-braces-around-statements.ShortStatementLines
value: '1'
- key: google-readability-function-size.StatementThreshold
value: '800'
- key: google-readability-namespace-comments.ShortNamespaceLines
value: '10'
- key: google-readability-namespace-comments.SpacesBeforeComments
value: '2'
- key: llvm-else-after-return.WarnOnConditionVariables
value: '0'
- key: llvm-else-after-return.WarnOnUnfixable
value: '0'
- key: llvm-qualified-auto.AddConstToQualified
value: '0'
- key: modernize-loop-convert.MaxCopySize
value: '16'
- key: modernize-loop-convert.MinConfidence
value: reasonable
- key: modernize-loop-convert.NamingStyle
value: CamelCase
- key: modernize-pass-by-value.IncludeStyle
value: llvm
- key: modernize-replace-auto-ptr.IncludeStyle
value: llvm
- key: modernize-use-nullptr.NullMacros
value: 'NULL'
- key: readability-identifier-length.IgnoredParameterNames
value: 'mr|os'
- key: readability-identifier-length.IgnoredVariableNames
value: 'mr|_'
- key: readability-function-cognitive-complexity.IgnoreMacros
value: '1'
- key: bugprone-easily-swappable-parameters.IgnoredParameterNames
value: 'alignment'
...
22 changes: 11 additions & 11 deletions benchmarks/cuda_stream_pool/cuda_stream_pool_bench.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2020, NVIDIA CORPORATION.
* Copyright (c) 2020-2021, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -14,36 +14,36 @@
* limitations under the License.
*/

#include <benchmark/benchmark.h>

#include <rmm/cuda_stream_pool.hpp>
#include <rmm/detail/error.hpp>

#include <cuda_runtime_api.h>

#include <benchmark/benchmark.h>

#include <stdexcept>

static void BM_StreamPoolGetStream(benchmark::State& state)
{
rmm::cuda_stream_pool stream_pool{};

for (auto _ : state) {
auto s = stream_pool.get_stream();
cudaStreamQuery(s.value());
for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores)
auto stream = stream_pool.get_stream();
cudaStreamQuery(stream.value());
}

state.SetItemsProcessed(state.iterations());
state.SetItemsProcessed(static_cast<int64_t>(state.iterations()));
}
BENCHMARK(BM_StreamPoolGetStream)->Unit(benchmark::kMicrosecond);

static void BM_CudaStreamClass(benchmark::State& state)
{
for (auto _ : state) {
auto s = rmm::cuda_stream{};
cudaStreamQuery(s.view().value());
for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores)
auto stream = rmm::cuda_stream{};
cudaStreamQuery(stream.view().value());
}

state.SetItemsProcessed(state.iterations());
state.SetItemsProcessed(static_cast<int64_t>(state.iterations()));
}
BENCHMARK(BM_CudaStreamClass)->Unit(benchmark::kMicrosecond);

Expand Down
30 changes: 18 additions & 12 deletions benchmarks/device_uvector/device_uvector_bench.cu
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2020, NVIDIA CORPORATION.
* Copyright (c) 2020-2021, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -14,33 +14,39 @@
* limitations under the License.
*/

#include <benchmark/benchmark.h>

#include <cuda_runtime_api.h>
#include <rmm/device_uvector.hpp>
#include <rmm/device_vector.hpp>
#include <rmm/mr/device/cuda_memory_resource.hpp>
#include <rmm/mr/device/per_device_resource.hpp>
#include <rmm/mr/device/pool_memory_resource.hpp>

#include <benchmark/benchmark.h>

#include <cuda_runtime_api.h>

static void BM_UvectorSizeConstruction(benchmark::State& state)
{
rmm::mr::cuda_memory_resource cuda_mr{};
rmm::mr::pool_memory_resource<rmm::mr::cuda_memory_resource> mr{&cuda_mr};
rmm::mr::set_current_device_resource(&mr);

for (auto _ : state) {
for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores)
rmm::device_uvector<int32_t> vec(state.range(0), rmm::cuda_stream_view{});
cudaDeviceSynchronize();
}

state.SetItemsProcessed(state.iterations());
state.SetItemsProcessed(static_cast<int64_t>(state.iterations()));

rmm::mr::set_current_device_resource(nullptr);
}

const auto range_multiplier{10};
const auto range_start{10'000};
const auto range_end{1'000'000'000};

BENCHMARK(BM_UvectorSizeConstruction)
->RangeMultiplier(10)
->Range(10'000, 1'000'000'000)
->RangeMultiplier(range_multiplier)
->Range(range_start, range_end)
harrism marked this conversation as resolved.
Show resolved Hide resolved
->Unit(benchmark::kMicrosecond);

static void BM_ThrustVectorSizeConstruction(benchmark::State& state)
Expand All @@ -49,19 +55,19 @@ static void BM_ThrustVectorSizeConstruction(benchmark::State& state)
rmm::mr::pool_memory_resource<rmm::mr::cuda_memory_resource> mr{&cuda_mr};
rmm::mr::set_current_device_resource(&mr);

for (auto _ : state) {
for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores)
rmm::device_vector<int32_t> vec(state.range(0));
cudaDeviceSynchronize();
}

state.SetItemsProcessed(state.iterations());
state.SetItemsProcessed(static_cast<int64_t>(state.iterations()));

rmm::mr::set_current_device_resource(nullptr);
}

BENCHMARK(BM_ThrustVectorSizeConstruction)
->RangeMultiplier(10)
->Range(10'000, 1'000'000'000)
->RangeMultiplier(range_multiplier)
->Range(range_start, range_end)
->Unit(benchmark::kMicrosecond);

BENCHMARK_MAIN();
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@

#include <benchmarks/utilities/cxxopts.hpp>

#include <benchmark/benchmark.h>

#include <rmm/cuda_stream.hpp>
#include <rmm/cuda_stream_pool.hpp>
#include <rmm/device_uvector.hpp>
Expand All @@ -31,15 +29,18 @@

#include <cuda_runtime_api.h>

#include <benchmark/benchmark.h>

#include <cstddef>

__global__ void compute_bound_kernel(int64_t* out)
{
clock_t clock_begin = clock64();
clock_t clock_current = clock_begin;
auto const million{1'000'000};

if (threadIdx.x == 0) {
while (clock_current - clock_begin < 1000000) {
if (threadIdx.x == 0) { // NOLINT(readability-static-accessed-through-instance)
Copy link
Contributor

Choose a reason for hiding this comment

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

wat. Is clang-tidy going to complain about every usage of threadIdx.x?

Copy link
Member Author

Choose a reason for hiding this comment

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

I am unsure about this. We can probably disable the rule for .cu files. In RMM there are so few .cu files that NOLINT isn't really a problem.

while (clock_current - clock_begin < million) {
clock_current = clock64();
}
}
Expand Down Expand Up @@ -69,26 +70,26 @@ static void run_test(std::size_t num_kernels,
}
}

static void BM_MultiStreamAllocations(benchmark::State& state, MRFactoryFunc factory)
static void BM_MultiStreamAllocations(benchmark::State& state, MRFactoryFunc const& factory)
{
auto mr = factory();

rmm::mr::set_current_device_resource(mr.get());

auto num_streams = state.range(0);
auto num_kernels = state.range(1);
auto do_prewarm = state.range(2);
bool do_prewarm = state.range(2) != 0;

auto stream_pool = rmm::cuda_stream_pool(num_streams);

if (do_prewarm) { run_prewarm(stream_pool, mr.get()); }

for (auto _ : state) {
for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores)
run_test(num_kernels, stream_pool, mr.get());
cudaDeviceSynchronize();
}

state.SetItemsProcessed(state.iterations() * num_kernels);
state.SetItemsProcessed(static_cast<int64_t>(state.iterations() * num_kernels));

rmm::mr::set_current_device_resource(nullptr);
}
Expand Down Expand Up @@ -124,7 +125,7 @@ static void benchmark_range(benchmark::internal::Benchmark* b)
->Unit(benchmark::kMicrosecond);
}

MRFactoryFunc get_mr_factory(std::string resource_name)
MRFactoryFunc get_mr_factory(std::string const& resource_name)
{
if (resource_name == "cuda") { return &make_cuda; }
#ifdef RMM_CUDA_MALLOC_ASYNC_SUPPORT
Expand All @@ -139,7 +140,7 @@ MRFactoryFunc get_mr_factory(std::string resource_name)
RMM_FAIL();
}

void declare_benchmark(std::string name)
void declare_benchmark(std::string const& name)
{
if (name == "cuda") {
BENCHMARK_CAPTURE(BM_MultiStreamAllocations, cuda, &make_cuda) //
Expand Down Expand Up @@ -176,7 +177,7 @@ void declare_benchmark(std::string name)
std::cout << "Error: invalid memory_resource name: " << name << std::endl;
}

void run_profile(std::string resource_name, int kernel_count, int stream_count, bool prewarm)
void run_profile(std::string const& resource_name, int kernel_count, int stream_count, bool prewarm)
{
auto mr_factory = get_mr_factory(resource_name);
auto mr = mr_factory();
Expand Down Expand Up @@ -228,7 +229,11 @@ int main(int argc, char** argv)
auto num_kernels = args["kernels"].as<int>();
auto num_streams = args["streams"].as<int>();
auto prewarm = args["warm"].as<bool>();
run_profile(resource_name, num_kernels, num_streams, prewarm);
try {
run_profile(resource_name, num_kernels, num_streams, prewarm);
} catch (std::exception const& e) {
std::cout << "Exception caught: " << e.what() << std::endl;
}
} else {
auto resource_names = std::vector<std::string>();

Expand Down
Loading