Skip to content

Commit

Permalink
Merge pull request #66 from lanl/bkk_warnings
Browse files Browse the repository at this point in the history 
Improve warnings
  • Loading branch information
@Yurlungur
Yurlungur authored Nov 20, 2024
2 parents b9e5d1d + 5bf92e8 commit 1e0442b
Show file tree
Hide file tree
Showing 9 changed files with 147 additions and 73 deletions.
8 changes: 6 additions & 2 deletions ports-of-call/array.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -194,9 +194,13 @@ struct array {

//! swap the array contents with another
PORTABLE_FUNCTION constexpr void swap(array &other) {
using std::swap;
for (size_type i = 0; i < N; ++i) {
swap(arr[i], other.arr[i]);
// C++20 makes std::swap constexpr, so this should probably be replaced
// at that point. For now, this is required if a user wants to call swap
// on a GPU.
T t = arr[i];
arr[i] = other.arr[i];
other.arr[i] = t;
}
}
};
Expand Down
46 changes: 24 additions & 22 deletions ports-of-call/portability.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,8 @@ void portableCopyToHost(T *const to, T const *const from, size_t const size_byte
}

template <typename Function>
void portableFor(const char *name, int start, int stop, Function function) {
void portableFor([[maybe_unused]] const char *name, int start, int stop,
Function function) {
#ifdef PORTABILITY_STRATEGY_KOKKOS
using policy = Kokkos::RangePolicy<>;
Kokkos::parallel_for(name, policy(start, stop), function);
Expand All @@ -165,8 +166,8 @@ void portableFor(const char *name, int start, int stop, Function function) {
}

template <typename Function>
void portableFor(const char *name, int starty, int stopy, int startx, int stopx,
Function function) {
void portableFor([[maybe_unused]] const char *name, int starty, int stopy, int startx,
int stopx, Function function) {
#ifdef PORTABILITY_STRATEGY_KOKKOS
using Policy2D = Kokkos::MDRangePolicy<Kokkos::Rank<2>>;
Kokkos::parallel_for(name, Policy2D({starty, startx}, {stopy, stopx}), function);
Expand All @@ -180,8 +181,8 @@ void portableFor(const char *name, int starty, int stopy, int startx, int stopx,
}

template <typename Function>
void portableFor(const char *name, int startz, int stopz, int starty, int stopy,
int startx, int stopx, Function function) {
void portableFor([[maybe_unused]] const char *name, int startz, int stopz, int starty,
int stopy, int startx, int stopx, Function function) {
#ifdef PORTABILITY_STRATEGY_KOKKOS
using Policy3D = Kokkos::MDRangePolicy<Kokkos::Rank<3>>;
Kokkos::parallel_for(name, Policy3D({startz, starty, startx}, {stopz, stopy, stopx}),
Expand All @@ -198,8 +199,9 @@ void portableFor(const char *name, int startz, int stopz, int starty, int stopy,
}

template <typename Function>
void portableFor(const char *name, int starta, int stopa, int startz, int stopz,
int starty, int stopy, int startx, int stopx, Function function) {
void portableFor([[maybe_unused]] const char *name, int starta, int stopa, int startz,
int stopz, int starty, int stopy, int startx, int stopx,
Function function) {
#ifdef PORTABILITY_STRATEGY_KOKKOS
using Policy4D = Kokkos::MDRangePolicy<Kokkos::Rank<4>>;
Kokkos::parallel_for(
Expand All @@ -219,9 +221,9 @@ void portableFor(const char *name, int starta, int stopa, int startz, int stopz,
}

template <typename Function>
void portableFor(const char *name, int startb, int stopb, int starta, int stopa,
int startz, int stopz, int starty, int stopy, int startx, int stopx,
Function function) {
void portableFor([[maybe_unused]] const char *name, int startb, int stopb, int starta,
int stopa, int startz, int stopz, int starty, int stopy, int startx,
int stopx, Function function) {
#ifdef PORTABILITY_STRATEGY_KOKKOS
using Policy5D = Kokkos::MDRangePolicy<Kokkos::Rank<5>>;
Kokkos::parallel_for(name,
Expand All @@ -244,8 +246,8 @@ void portableFor(const char *name, int startb, int stopb, int starta, int stopa,
}

template <typename Function, typename T>
void portableReduce(const char *name, int start, int stop, Function function,
T &reduced) {
void portableReduce([[maybe_unused]] const char *name, int start, int stop,
Function function, T &reduced) {
#ifdef PORTABILITY_STRATEGY_KOKKOS
using Policy = Kokkos::RangePolicy<>;
Kokkos::parallel_reduce(name, Policy(start, stop), function, reduced);
Expand All @@ -257,8 +259,8 @@ void portableReduce(const char *name, int start, int stop, Function function,
}

template <typename Function, typename T>
void portableReduce(const char *name, int starty, int stopy, int startx, int stopx,
Function function, T &reduced) {
void portableReduce([[maybe_unused]] const char *name, int starty, int stopy, int startx,
int stopx, Function function, T &reduced) {
#ifdef PORTABILITY_STRATEGY_KOKKOS
using Policy2D = Kokkos::MDRangePolicy<Kokkos::Rank<2>>;
Kokkos::parallel_reduce(name, Policy2D({starty, startx}, {stopy, stopx}), function,
Expand All @@ -273,8 +275,8 @@ void portableReduce(const char *name, int starty, int stopy, int startx, int sto
}

template <typename Function, typename T>
void portableReduce(const char *name, int startz, int stopz, int starty, int stopy,
int startx, int stopx, Function function, T &reduced) {
void portableReduce([[maybe_unused]] const char *name, int startz, int stopz, int starty,
int stopy, int startx, int stopx, Function function, T &reduced) {
#ifdef PORTABILITY_STRATEGY_KOKKOS
using Policy3D = Kokkos::MDRangePolicy<Kokkos::Rank<3>>;
Kokkos::parallel_reduce(name, Policy3D({startz, starty, startx}, {stopz, stopy, stopx}),
Expand All @@ -291,9 +293,9 @@ void portableReduce(const char *name, int startz, int stopz, int starty, int sto
}

template <typename Function, typename T>
void portableReduce(const char *name, int starta, int stopa, int startz, int stopz,
int starty, int stopy, int startx, int stopx, Function function,
T &reduced) {
void portableReduce([[maybe_unused]] const char *name, int starta, int stopa, int startz,
int stopz, int starty, int stopy, int startx, int stopx,
Function function, T &reduced) {
#ifdef PORTABILITY_STRATEGY_KOKKOS
using Policy4D = Kokkos::MDRangePolicy<Kokkos::Rank<4>>;
Kokkos::parallel_reduce(
Expand All @@ -313,9 +315,9 @@ void portableReduce(const char *name, int starta, int stopa, int startz, int sto
}

template <typename Function, typename T>
void portableReduce(const char *name, int startb, int stopb, int starta, int stopa,
int startz, int stopz, int starty, int stopy, int startx, int stopx,
Function function, T &reduced) {
void portableReduce([[maybe_unused]] const char *name, int startb, int stopb, int starta,
int stopa, int startz, int stopz, int starty, int stopy, int startx,
int stopx, Function function, T &reduced) {
#ifdef PORTABILITY_STRATEGY_KOKKOS
using Policy5D = Kokkos::MDRangePolicy<Kokkos::Rank<5>>;
Kokkos::parallel_reduce(name,
Expand Down
36 changes: 24 additions & 12 deletions ports-of-call/span.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,16 @@ namespace detail {
using std::data;
using std::size;

// type-safe check against 0 (prevents warnings about comparing an unsigned against 0)
template <typename T, std::enable_if_t<std::is_unsigned<T>::value, bool> = true>
PORTABLE_FUNCTION constexpr bool check_nonnegative(const T) {
return true;
}
template <typename T, std::enable_if_t<!std::is_unsigned<T>::value, bool> = true>
PORTABLE_FUNCTION constexpr bool check_nonnegative(const T t) {
return t >= 0;
}

// object to handle storage of span
template <class E, std::size_t S>
struct span_storage {
Expand Down Expand Up @@ -195,7 +205,8 @@ class span {
// explicit: extent != dynamic_extent
// NB: iterator concepts to further restrict overload resolution
constexpr span(pointer ptr, size_type count) : storage_(ptr, count) {
span_EXPECTS((ptr == nullptr && count == 0) || (ptr != nullptr && count >= 0));
span_EXPECTS((ptr == nullptr && count == 0) ||
(ptr != nullptr && detail::check_nonnegative(count)));
}

// constructs a span that is a view over the range [first, last)
Expand All @@ -205,7 +216,7 @@ class span {
// NB: iterator concepts to further restrict overload resolution
constexpr span(pointer first_elem, pointer last_elem)
: storage_(first_elem, last_elem - first_elem) {
span_EXPECTS(last_elem - first_elem >= 0);
span_EXPECTS(detail::check_nonnegative(last_elem - first_elem));
}

// constructs a span that is a view over arr
Expand Down Expand Up @@ -272,25 +283,25 @@ class span {
// https://en.cppreference.com/w/cpp/container/span/first
template <std::size_t Count>
constexpr span<element_type, Count> first() const {
span_EXPECTS(Count >= 0 && Count <= size());
span_EXPECTS(detail::check_nonnegative(Count) && Count <= size());
return {data(), Count};
}

constexpr span<element_type, dynamic_extent> first(size_type count) const {
span_EXPECTS(count >= 0 && count <= size());
span_EXPECTS(detail::check_nonnegative(count) && count <= size());
return {data(), count};
}

// obtains a span that is a view over the last Count elements of this span
// https://en.cppreference.com/w/cpp/container/span/last
template <std::size_t Count>
constexpr span<element_type, Count> last() const {
span_EXPECTS(Count >= 0 && Count <= size());
span_EXPECTS(detail::check_nonnegative(Count) && Count <= size());
return {data() + (size() - Count), Count};
}

constexpr span<element_type, dynamic_extent> last(size_type count) const {
span_EXPECTS(count >= 0 && count <= size());
span_EXPECTS(detail::check_nonnegative(count) && count <= size());
return {data() + (size() - count), count};
}

Expand All @@ -305,16 +316,17 @@ class span {

template <std::size_t Offset, std::size_t Count = dynamic_extent>
constexpr subspan_return_t<Offset, Count> subspan() const {
span_EXPECTS((Offset >= 0 && Offset <= size()) &&
(Count == dynamic_extent || (Count >= 0 && Count + Offset <= size())));
span_EXPECTS((detail::check_nonnegative(Offset) && Offset <= size()) &&
(Count == dynamic_extent ||
(detail::check_nonnegative(Count) && Count + Offset <= size())));
return {data() + Offset, Count != dynamic_extent ? Count : size() - Offset};
}

constexpr span<element_type, dynamic_extent>
subspan(size_type offset, size_type count = dynamic_extent) const {
span_EXPECTS((offset >= 0 && offset <= size()) &&
span_EXPECTS((detail::check_nonnegative(offset) && offset <= size()) &&
(count == static_cast<size_type>(dynamic_extent) ||
(count >= 0 && count + offset <= size())));
(detail::check_nonnegative(count) && count + offset <= size())));

return {data() + offset, count == dynamic_extent ? size() - offset : count};
}
Expand All @@ -337,7 +349,7 @@ class span {

// [span.element_access]--
constexpr reference operator[](size_type idx) const {
span_EXPECTS(idx >= 0 && idx < size());
span_EXPECTS(detail::check_nonnegative(idx) && idx < size());
return *(data() + idx);
}

Expand Down Expand Up @@ -381,7 +393,7 @@ span(const std::array<T, N> &) -> span<const T, N>;

template <class Container>
span(Container &) -> span<typename std::remove_reference<
decltype(*detail::data(std::declval<Container &>()))>::type>;
decltype(*detail::data(std::declval<Container &>()))>::type>;

template <class Container>
span(const Container &) -> span<const typename Container::value_type>;
Expand Down
35 changes: 35 additions & 0 deletions test/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,41 @@ elseif (PORTS_OF_CALL_TEST_PORTABILITY_STRATEGY STREQUAL "Cuda")
target_compile_definitions(portsofcall_iface INTERFACE PORTABILITY_STRATEGY_CUDA)
endif()

if(NOT PORTS_OF_CALL_SILENCE_WARNINGS)
# turn on strict warnings and error
# NOTE: The ROCm 6.2.1 installation on some machines (RZAdams and RZVernal at least) will
# inject a `--gcc-toolchain` command-line argument into Clang, which is a deprecated
# option. Clang will warn about unused command-line arguments, so you get multiple
# instances of that warning. Then `-Werror` promotes those to errors. We can turn these
# back to non-fatal warnings with `-Wno-error=unused-command-line-argument` or we can
# turn off that particular warning with `-Wno-unused-command-line-argument`.
# TODO: find comiler flags for non-GNU compatible compilers
target_compile_options(portsofcall_iface INTERFACE
$<$<IN_LIST:${CMAKE_CXX_COMPILER_ID},GNU>:-Werror;-Wall;-pedantic-errors;-Wunused-parameter>
$<$<IN_LIST:${CMAKE_CXX_COMPILER_ID},AppleClang>:-Werror;-Wall;-Wunused-parameter>
$<$<NOT:$<BOOL:${WIN32}>>:$<$<IN_LIST:${CMAKE_CXX_COMPILER_ID},Clang>:-Werror;-Wall;-Wno-unused-command-line-argument>>
$<$<BOOL:${WIN32}>:$<$<IN_LIST:${CMAKE_CXX_COMPILER_ID},Clang>:-Wall;-Wno-c++98-compat;-Wno-c++98-compat-pedantic>>
$<$<IN_LIST:${CMAKE_CXX_COMPILER_ID},Intel>:-Werror;-w2;-Wunused-variable;-Wunused-parameter;-diag-disable=remark>
$<$<IN_LIST:${CMAKE_CXX_COMPILER_ID},PGI>:>
$<$<IN_LIST:${CMAKE_CXX_COMPILER_ID},MSVC>:/permissive->
$<$<IN_LIST:${CMAKE_CXX_COMPILER_ID},XL>:>
)
else()
# turn off warnings
# TODO: find comiler flags for non-GNU compatible compilers
target_compile_options(portsofcall_iface INTERFACE
$<$<IN_LIST:${CMAKE_CXX_COMPILER_ID},GNU>:-w>
$<$<IN_LIST:${CMAKE_CXX_COMPILER_ID},AppleClang>:>
$<$<NOT:$<BOOL:${WIN32}>>:$<$<IN_LIST:${CMAKE_CXX_COMPILER_ID},Clang>:>>
$<$<BOOL:${WIN32}>:$<$<IN_LIST:${CMAKE_CXX_COMPILER_ID},Clang>:>>
$<$<IN_LIST:${CMAKE_CXX_COMPILER_ID},Intel>:>
$<$<IN_LIST:${CMAKE_CXX_COMPILER_ID},PGI>:>
$<$<IN_LIST:${CMAKE_CXX_COMPILER_ID},MSVC>:>
$<$<IN_LIST:${CMAKE_CXX_COMPILER_ID},XL>:>
)
endif()


target_link_libraries(portsofcall_iface INTERFACE Catch2::Catch2)

add_executable(test_portsofcall test_main.cpp)
Expand Down
16 changes: 8 additions & 8 deletions test/test_array.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ TEST_CASE("array begins and ends", "[array]") {
using std::end;

SECTION("with non-const array") {
array<int, 3> arr{{1, 2, 3}};
[[maybe_unused]] array<int, 3> arr{{1, 2, 3}};

CHECK(std::is_same<int *, decltype(begin(arr))>::value);
CHECK(std::is_same<int const *, decltype(cbegin(arr))>::value);
Expand All @@ -129,7 +129,7 @@ TEST_CASE("array begins and ends", "[array]") {
}

SECTION("with const array") {
array<int, 3> const arr{{1, 2, 3}};
[[maybe_unused]] array<int, 3> const arr{{1, 2, 3}};

CHECK(std::is_same<int const *, decltype(begin(arr))>::value);
CHECK(std::is_same<int const *, decltype(cbegin(arr))>::value);
Expand All @@ -139,13 +139,13 @@ TEST_CASE("array begins and ends", "[array]") {
}

SECTION("with non-const zero-sized array") {
array<int, 0> arr;
[[maybe_unused]] array<int, 0> arr;

CHECK(begin(arr) == end(arr));
}

SECTION("with const zero-sized array") {
array<int, 0> const arr{};
[[maybe_unused]] array<int, 0> const arr{};

CHECK(begin(arr) == end(arr));
}
Expand Down Expand Up @@ -208,7 +208,7 @@ TEST_CASE("array sizes", "[array]") {
TEST_CASE("array fill (GPU)", "[array][GPU]") {
constexpr std::size_t N = 42;
std::size_t count = 0;
auto func = PORTABLE_LAMBDA(const int i, std::size_t &count) {
auto func = PORTABLE_LAMBDA(const int /*i*/, std::size_t &count) {
constexpr double value = 3.14;
array<double, N> arr;
arr.fill(value);
Expand Down Expand Up @@ -260,21 +260,21 @@ TEST_CASE("array swap", "[array]") {
}

TEST_CASE("array tuple_size", "[array]") {
array<double, 5> arr;
[[maybe_unused]] array<double, 5> arr;

CHECK(std::tuple_size<decltype(arr)>::value == 5);
}

TEST_CASE("array tuple_element", "[array]") {
struct Foo {};

array<Foo, 3> foos;
[[maybe_unused]] array<Foo, 3> foos;

CHECK(std::is_same<Foo, std::tuple_element<1, decltype(foos)>::type>::value);
}

TEST_CASE("make_array function", "[array]") {
auto arr = make_array(1.0, 2.0, 3.0);
[[maybe_unused]] auto arr = make_array(1.0, 2.0, 3.0);

CHECK(std::is_same<decltype(arr), array<double, 3>>::value);
}
4 changes: 2 additions & 2 deletions test/test_portability.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ TEST_CASE("portableCopy works with all portability strategies", "[portableCopy]"
int sum{0};
portableReduce(
"check portableCopy", 0, N, 0, 0, 0, 0,
PORTABLE_LAMBDA(const int &i, const int &j, const int &k, int &isum) {
PORTABLE_LAMBDA(const int &i, const int & /*j*/, const int & /*k*/, int &isum) {
if (a[i] != index_func(i)) {
isum += 1;
}
Expand All @@ -121,7 +121,7 @@ TEST_CASE("portableCopy works with all portability strategies", "[portableCopy]"

// count elements that don't match reference
sum = 0;
for (int i = 0; i < N; ++i) {
for (size_t i = 0; i < N; ++i) {
if (b[i] != index_func(i)) {
sum += 1;
}
Expand Down
5 changes: 2 additions & 3 deletions test/test_span.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,7 @@ constexpr void generate(ForwardIt first, ForwardIt last, Generator g) {
}

template <class T, std::size_t N>
[[nodiscard]]
constexpr auto slide(span<T, N> s, std::size_t offset, std::size_t width) {
[[nodiscard]] constexpr auto slide(span<T, N> s, std::size_t offset, std::size_t width) {
return s.subspan(offset, offset + width <= s.size() ? width : 0U);
}

Expand Down Expand Up @@ -569,7 +568,7 @@ TEST_CASE("span portability", "[PortsOfCall::span]") {
constexpr Real d_gp = 2. * pi / static_cast<Real>(N);

std::vector<Real> h_gp(N);
for (auto i = 0; i < N; ++i) {
for (std::size_t i = 0; i < N; ++i) {
h_gp[i] = -pi + static_cast<Real>(i) * d_gp;
}

Expand Down
Loading

0 comments on commit 1e0442b

Please sign in to comment.