Skip to content

[SYCL] logical operation return type is boolean #17239

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.

Sign up for GitHub

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
Apr 22, 2025
Merged

[SYCL] logical operation return type is boolean #17239

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
48153fb
[SYCL] logical operation return type is boolean
dklochkov-emb Feb 24, 2025
b6df1d9
[SYCL] update test to check void types
dklochkov-emb Mar 6, 2025
0c4d352
[SYCL] simplify changes
dklochkov-emb Mar 24, 2025
b1d3543
[SYCL] crate separate negative test for logical operations with type …
dklochkov-emb Mar 27, 2025
df2fcb2
[SYCL] update formatting
dklochkov-emb Mar 27, 2025
fe3b943
[SYCL] simplify expression to test logfical operations with void types
dklochkov-emb Apr 14, 2025
89462df
[SYCL] avoid true in test expressions
dklochkov-emb Apr 14, 2025
025456a
[SYCL] do not create separate test for void types
dklochkov-emb Apr 14, 2025
b800256
[SYCL] use -fsynatx-only
dklochkov-emb Apr 14, 2025
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
27 changes: 22 additions & 5 deletions sycl/include/sycl/functional.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,34 @@ template <typename T = void> using bit_xor = std::bit_xor<T>;

// std:logical_and/std::logical_or with a non-void type returns bool,
// sycl requires returning T.
template <typename T = void> struct logical_and {
template <typename T = void>
struct logical_and
#ifdef __INTEL_PREVIEW_BREAKING_CHANGES
: std::logical_and<T>
#endif
{
#ifndef __INTEL_PREVIEW_BREAKING_CHANGES
T operator()(const T &lhs, const T &rhs) const { return lhs && rhs; }
};
#endif
}; // namespace _V1

#ifndef __INTEL_PREVIEW_BREAKING_CHANGES
template <> struct logical_and<void> : std::logical_and<void> {};
#endif

template <typename T = void> struct logical_or {
template <typename T = void>
struct logical_or
#ifdef __INTEL_PREVIEW_BREAKING_CHANGES
: std::logical_or<T>
#endif
{
#ifndef __INTEL_PREVIEW_BREAKING_CHANGES
T operator()(const T &lhs, const T &rhs) const { return lhs || rhs; }
};

#endif
}; // namespace sycl
#ifndef __INTEL_PREVIEW_BREAKING_CHANGES
template <> struct logical_or<void> : std::logical_or<void> {};
#endif

// sycl::minimum definition should be consistent with std::min
template <typename T = void> struct minimum {
Expand Down
35 changes: 35 additions & 0 deletions sycl/test/basic_tests/logical_operations.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// RUN: %clang -fpreview-breaking-changes -fsycl -o - %s
Copy link
Contributor

@aelovikov-intel aelovikov-intel Apr 11, 2025

Choose a reason for hiding this comment

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

@steffenlarsen , do we have a reasonable use-case for a build without preview enabled?

Copy link
Contributor

@steffenlarsen steffenlarsen Apr 14, 2025

Choose a reason for hiding this comment

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

Not really, I don't think. Since we can do -fsyntax-only (and maybe -fsycl-host-only?), the compilation should be somewhat cheap, so a benefit of doing both is that we ensure the return type is preserved until preview-promotion. Additionally, it makes the distinction between preview and non-preview behavior clearer.

Copy link
Contributor

@aelovikov-intel aelovikov-intel Apr 14, 2025

Choose a reason for hiding this comment

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

What I meant is that we have this:

llvm/sycl/test-e2e/lit.cfg.py

Lines 379 to 386 in b38eb60

with test_env():
sp = subprocess.getstatusoutput(
config.dpcpp_compiler
+ " -fsycl -fpreview-breaking-changes "
+ check_preview_breaking_changes_file
)
if sp[0] == 0:
config.available_features.add("preview-breaking-changes-supported")

We should either guard the entire RUN-line here, or make sure that we always build with preview enabled and it's always supported.

Copy link
Contributor

@steffenlarsen steffenlarsen Apr 15, 2025

Choose a reason for hiding this comment

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

Ah, thank you for clarifying. I think either solution is fine. Doing REQUIRE: preview-breaking-changes-supported would be the cleaner option IMO.

// RUN: not %clang -fpreview-breaking-changes -fsycl -DTEST_VOID_TYPES -o - %s
// RUN: %clang -fsycl -o - %s
// RUN: not %clang -fsycl -DTEST_VOID_TYPES -o - %s

#include <cassert>
#include <sycl/functional.hpp>
#include <type_traits>

int main() {
const auto logicalAnd = sycl::logical_and<int>();
const auto logicalOr = sycl::logical_or<int>();
#ifdef __INTEL_PREVIEW_BREAKING_CHANGES
static_assert(std::is_same_v<decltype(logicalAnd(1, 2)), bool> == true);
static_assert(std::is_same_v<decltype(logicalOr(1, 2)), bool> == true);
#else
static_assert(std::is_same_v<decltype(logicalAnd(1, 2)), int> == true);
static_assert(std::is_same_v<decltype(logicalOr(1, 2)), int> == true);
#endif
const auto logicalAndVoid = sycl::logical_and<void>();
const auto logicalOrVoid = sycl::logical_or<void>();

static_assert(std::is_same_v<decltype(logicalAndVoid(1, 2)), bool> == true);
static_assert(std::is_same_v<decltype(logicalOrVoid(1, 2)), bool> == true);

#ifdef TEST_VOID_TYPES
static_assert(std::is_same_v<decltype(logicalAndVoid(static_cast<void>(1),
static_cast<void>(2))),
bool> == true);
static_assert(std::is_same_v<decltype(logicalOrVoid(static_cast<void>(1),
static_cast<void>(2))),
bool> == true);
#endif
return 0;
}
Loading