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.

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

[SYCL][E2E] Add logic to react to REQUIRED/UNSUPPORTED in build-only #16725

Draft
wants to merge 11 commits into
base: sycl
Choose a base branch
from
+343 −86
Draft

[SYCL][E2E] Add logic to react to REQUIRED/UNSUPPORTED in build-only #16725

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
123b4ac
Add method to set triple + react to `REQUIRES` in build-only
ayylol Jan 21, 2025
cdd3aa1
Address comments related to new expressions
ayylol Jan 22, 2025
0be2b74
Rename triples to build_target where appropriate
ayylol Jan 22, 2025
5a2b31f
Address 2 more comments
ayylol Jan 22, 2025
9a5f6c8
Re-add `REQUIRES: build-and-run-mode` for tests failing on run-only
ayylol Jan 22, 2025
d95ce2f
Add final_unknown_value as defaulted param in `getMatchedFromList`
ayylol Jan 22, 2025
a917988
Add test to run E2EExpr unit tests, and move unit tests to separate file
ayylol Jan 23, 2025
184d9d9
Simplify code by using build_targets set throughout execute function
ayylol Jan 23, 2025
d1f8409
Add requires linux on E2EExpr tests
ayylol Jan 23, 2025
cff4341
Move E2EExpr unit tests back into E2EExpr.py
ayylol Jan 23, 2025
51c46b4
Format changes
ayylol Jan 23, 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
1 change: 0 additions & 1 deletion sycl/test-e2e/Adapters/cuda_queue_priority.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// REQUIRES: gpu, cuda, cuda_dev_kit
// REQUIRES: build-and-run-mode
// RUN: %{build} %cuda_options -o %t.out
// RUN: %{run} %t.out
//
Expand Down
1 change: 0 additions & 1 deletion sycl/test-e2e/Adapters/dll-detach-order.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// REQUIRES: windows
// REQUIRES: build-and-run-mode
// RUN: env SYCL_UR_TRACE=-1 sycl-ls | FileCheck %s

// ensure that the adapters are detached AFTER urLoaderTearDown is done
Expand Down
3 changes: 1 addition & 2 deletions sycl/test-e2e/AmdNvidiaJIT/kernel_and_bundle.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
// UNSUPPORTED: windows
// REQUIRES: cuda || hip
// REQUIRES: build-and-run-mode
// REQUIRES: target-nvidia || target-amd

// This test relies on debug output from a pass, make sure that the compiler
// can generate it.
Expand Down
1 change: 0 additions & 1 deletion sycl/test-e2e/Basic/windows_version_agnostic_sycl_lib.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// REQUIRES: windows
// REQUIRES: build-and-run-mode

// RUN: %clangxx --driver-mode=cl /std:c++17 /EHsc %sycl_include -I%opencl_include_dir %s -o %t.out /link /defaultlib:%sycl_static_libs_dir/sycl.lib
// RUN: %{run} %t.out
Expand Down
1 change: 0 additions & 1 deletion sycl/test-e2e/Compression/no_zstd_warning.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// using --offload-compress without zstd should throw an error.
// REQUIRES: !zstd
// REQUIRES: build-and-run-mode
// RUN: not %{build} %O0 -g --offload-compress %S/Inputs/single_kernel.cpp -o %t_compress.out 2>&1 | FileCheck %s
// CHECK: '--offload-compress' option is specified but zstd is not available. The device image will not be compressed.
1 change: 0 additions & 1 deletion sycl/test-e2e/DeviceLib/math_fp64_windows_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// REQUIRES: aspect-fp64, windows
// REQUIRES: build-and-run-mode

// DEFINE: %{mathflags} = %if cl_options %{/clang:-fno-fast-math%} %else %{-fno-fast-math%}

Expand Down
1 change: 0 additions & 1 deletion sycl/test-e2e/DeviceLib/math_windows_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// REQUIRES: windows
// REQUIRES: build-and-run-mode

// TODO: Add hypotf case back when the missing symbol is fixed.

Expand Down
219 changes: 219 additions & 0 deletions sycl/test-e2e/E2EExpr.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,219 @@
from lit.BooleanExpression import BooleanExpression


class E2EExpr(BooleanExpression):
build_specific_features = {
"build-and-run-mode",
"target-spir",
"target-nvidia",
"target-amd",
"target-native_cpu",
"linux",
"system-linux",
"windows",
"system-windows",
"enable-perf-tests",
"opencl_icd",
"cuda_dev_kit",
"zstd",
"vulkan",
"true",
"false",
}

def __init__(self, string, variables, build_only_mode, findal_unknown_value):
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
def __init__(self, string, variables, build_only_mode, findal_unknown_value):
def __init__(self, string, variables, build_only_mode, final_unknown_value):

here and everywhere below.

BooleanExpression.__init__(self, string, variables)
self.build_only_mode = build_only_mode
self.unknown = False
self.findal_unknown_value = findal_unknown_value

@staticmethod
def evaluate(string, variables, build_only_mode, final_unknown_value=True):
"""
string: Expression to evaluate
variables: variables that evaluate to true
build_only_mode: if true enables unknown values
findal_unknown_value: findal boolean result if evaluation results in `unknown`
"""
try:
parser = E2EExpr(
string, set(variables), build_only_mode, final_unknown_value
)
return parser.parseAll()
except ValueError as e:
raise ValueError(str(e) + ("\nin expression: %r" % string))

def parseMATCH(self):
token = self.token
BooleanExpression.parseMATCH(self)
if token not in self.build_specific_features and self.build_only_mode:
self.unknown = True
else:
self.unknown = False

def parseAND(self):
self.parseNOT()
while self.accept("&&"):
left = self.value
left_unknown = self.unknown
self.parseNOT()
right = self.value
right_unknown = self.unknown
self.value = left and right
# Unknown if both are unknown or if one is true and the other is unknown
self.unknown = (
(left_unknown and right_unknown)
or (left_unknown and right)
or (left and right_unknown)
)

def parseOR(self):
self.parseAND()
while self.accept("||"):
left = self.value
left_unknown = self.unknown
self.parseAND()
right = self.value
right_unknown = self.unknown
self.value = left or right
# Unknown if both are unknown or if one is false and the other is unknown
self.unknown = (
(left_unknown and right_unknown)
or (left_unknown and not right)
or (not left and right_unknown)
)

def parseAll(self):
self.token = next(self.tokens)
self.parseOR()
self.expect(BooleanExpression.END)
return self.findal_unknown_value if self.unknown else self.value


import unittest


class TestE2EExpr(unittest.TestCase):
def test_basic(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd like to see this:

  BuildOnly = True
  BuildAndRun = False
  RequiresDirective = True
  UnsupportedOrXFAILDirective = False
  self.assertTrue(E2EExpr.evaluate(<expr>, <features>, BuildOnly, RequiresDirective)

And now that I spelled it out, it can be even better (C++-like syntax, names to be improved):

   auto BuildOnlyRequires = [](auto expr, auto features) { ... };
   ...
   self.assertTrue(BuildOnlyRequires("linux", {"linux", "rt_feature"}))
   self.assertFalse(BuildOnlyUnsupported("rt_feature", {}))

Also, since we want these name improvements in the tests, we might as well want to propagate them all the way to the interfaces somehow.

# Non build-only expressions should work the same
self.assertTrue(E2EExpr.evaluate("linux", {"linux", "rt_feature"}, False))
self.assertTrue(E2EExpr.evaluate("rt_feature", {"linux", "rt_feature"}, False))
self.assertFalse(
E2EExpr.evaluate(
"another_aspect && rt_feature", {"linux", "rt_feature"}, False
)
)
# build-only expressions with no unknowns should work the same
self.assertTrue(
E2EExpr.evaluate("linux", {"linux"}, True, final_unknown_value=False)
)
self.assertFalse(
E2EExpr.evaluate(
"linux && windows", {"linux"}, True, final_unknown_value=True
)
)
self.assertTrue(
E2EExpr.evaluate(
"!(windows || zstd)", {"linux"}, True, final_unknown_value=False
)
)
# build-only expressions where unknown affects the resulting value
self.assertTrue(
E2EExpr.evaluate("rt_feature", {}, True, final_unknown_value=True)
)
self.assertFalse(
E2EExpr.evaluate("rt_feature", {}, True, final_unknown_value=False)
)
self.assertTrue(
E2EExpr.evaluate(
"rt_feature", {"rt_feature"}, True, final_unknown_value=True
Copy link
Contributor

Choose a reason for hiding this comment

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

should we assert/raise an error instead?

)
)
self.assertFalse(
E2EExpr.evaluate(
"rt_feature", {"rt_feature"}, True, final_unknown_value=False
)
)
self.assertFalse(
E2EExpr.evaluate("!rt_feature", {}, True, final_unknown_value=False)
)
self.assertFalse(
E2EExpr.evaluate("!!rt_feature", {}, True, final_unknown_value=False)
)
self.assertTrue(
E2EExpr.evaluate(
"windows || rt_feature", {"linux"}, True, final_unknown_value=True
)
)
self.assertFalse(
E2EExpr.evaluate(
"windows || rt_feature", {"linux"}, True, final_unknown_value=False
)
)
self.assertTrue(
E2EExpr.evaluate(
"linux && rt_feature", {"linux"}, True, final_unknown_value=True
)
)
self.assertFalse(
E2EExpr.evaluate(
"linux && rt_feature", {"linux"}, True, final_unknown_value=False
)
)
self.assertTrue(
E2EExpr.evaluate(
"linux && !(windows || rt_feature)",
Copy link
Contributor

Choose a reason for hiding this comment

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

I have hard time mapping that to a possible REQURIES directive. I think changing "windows" to something non-OS might help, but even then I'm not sure what would be the best example. Same for the next one.

{"linux"},
True,
final_unknown_value=True,
)
)
self.assertFalse(
E2EExpr.evaluate(
"linux && !(windows || rt_feature)",
{"linux"},
True,
final_unknown_value=False,
)
)
# build-only expressions where unknown does not affect the resulting value
self.assertTrue(
E2EExpr.evaluate(
"linux || rt_feature", {"linux"}, True, final_unknown_value=True
)
)
self.assertTrue(
E2EExpr.evaluate(
"linux || rt_feature", {"linux"}, True, final_unknown_value=False
)
)
self.assertFalse(
E2EExpr.evaluate(
"windows && rt_feature", {"linux"}, True, final_unknown_value=True
)
)
self.assertFalse(
E2EExpr.evaluate(
"windows && rt_feature", {"linux"}, True, final_unknown_value=False
)
)
self.assertFalse(
E2EExpr.evaluate(
"linux && (windows && rt_feature)",
Copy link
Contributor

Choose a reason for hiding this comment

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

change "windows" to something that can make sense in real world. Same in the next one.

{"linux"},
True,
final_unknown_value=True,
)
)
self.assertFalse(
E2EExpr.evaluate(
"linux && (windows && rt_feature)",
{"linux"},
True,
final_unknown_value=False,
)
)


if __name__ == "__main__":
unittest.main()
1 change: 0 additions & 1 deletion sycl/test-e2e/EnqueueNativeCommand/custom-command-cuda.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// RUN: %{build} -Wno-error=deprecated-declarations -o %t.out %cuda_options
// RUN: %{run} %t.out
// REQUIRES: cuda, cuda_dev_kit
// REQUIRES: build-and-run-mode

#include <cuda.h>

Expand Down
3 changes: 1 addition & 2 deletions sycl/test-e2e/EnqueueNativeCommand/custom-command-hip.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@
// we should set this with some variable instead.
// RUN: %{build} -Wno-error=deprecated-pragma -o %t.out -I%rocm_path/include -L%rocm_path/lib -lamdhip64
// RUN: %{run} %t.out
// REQUIRES: hip
// REQUIRES: build-and-run-mode
// REQUIRES: target-amd

#include <iostream>
#include <sycl/backend.hpp>
Expand Down
1 change: 0 additions & 1 deletion sycl/test-e2e/EnqueueNativeCommand/custom-command-multiple-dev-cuda.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// REQUIRES: cuda, cuda_dev_kit
// REQUIRES: build-and-run-mode
// RUN: %{build} -o %t.out %cuda_options
// RUN: %{run} %t.out

Expand Down
1 change: 0 additions & 1 deletion sycl/test-e2e/HostInteropTask/interop-task-cuda-buffer-migrate.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// REQUIRES: cuda, cuda_dev_kit
// REQUIRES: build-and-run-mode
//
// RUN: %{build} -o %t.out %cuda_options
// RUN: %{run} %t.out
Expand Down
1 change: 0 additions & 1 deletion sycl/test-e2e/HostInteropTask/interop-task-cuda.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// RUN: %{build} -o %t.out %cuda_options
// RUN: %{run} %t.out
// REQUIRES: cuda, cuda_dev_kit
// REQUIRES: build-and-run-mode

#include <iostream>
#include <sycl/backend.hpp>
Expand Down
3 changes: 1 addition & 2 deletions sycl/test-e2e/HostInteropTask/interop-task-hip.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@
// we should set this with some variable instead.
// RUN: %{build} -Wno-error=deprecated-pragma -Wno-error=deprecated-declarations -o %t.out -I%rocm_path/include -L%rocm_path/lib -lamdhip64
// RUN: %{run} %t.out
// REQUIRES: hip
// REQUIRES: build-and-run-mode
// REQUIRES: target-amd

#include <iostream>
#include <sycl/backend.hpp>
Expand Down
1 change: 0 additions & 1 deletion sycl/test-e2e/Matrix/joint_matrix_hip_gfx90a.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
// RUN: %{run} %t.out

// REQUIRES: arch-amd_gpu_gfx90a
// REQUIRES: build-and-run-mode

#include "joint_matrix_hip_apply.hpp"
#include "joint_matrix_hip_copy.hpp"
Expand Down
1 change: 0 additions & 1 deletion sycl/test-e2e/Matrix/joint_matrix_hip_half_gfx90a.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@

// REQUIRES: arch-amd_gpu_gfx90a
// REQUIRES: aspect-fp16
// REQUIRES: build-and-run-mode

#include "joint_matrix_hip_apply.hpp"
#include "joint_matrix_hip_copy.hpp"
Expand Down
3 changes: 1 addition & 2 deletions sycl/test-e2e/Matrix/joint_matrix_tensorcores_sm70.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@
//
//===----------------------------------------------------------------------===//

// REQUIRES: cuda
// REQUIRES: build-and-run-mode
// REQUIRES: target-nvidia
// RUN: %{build} -Xsycl-target-backend --cuda-gpu-arch=sm_70 -o %t.out
// RUN: %{run} %t.out
//
Expand Down
1 change: 0 additions & 1 deletion sycl/test-e2e/Matrix/runtime_query_hip_gfx90a.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
//===----------------------------------------------------------------------===//

// REQUIRES: arch-amd_gpu_gfx90a
// REQUIRES: build-and-run-mode
// RUN: %clangxx -fsycl -fsycl-targets=amdgcn-amd-amdhsa -Xsycl-target-backend=amdgcn-amd-amdhsa --offload-arch=gfx90a %s -o %t.out
// RUN: %{run} %t.out

Expand Down
1 change: 0 additions & 1 deletion sycl/test-e2e/OptionalKernelFeatures/is_compatible/is_compatible_amdgcn.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// REQUIRES: hip, opencl, gpu, cpu
// REQUIRES: build-and-run-mode

// RUN: %clangxx -fsycl -Xsycl-target-backend=amdgcn-amd-amdhsa --offload-arch=gfx906 -fsycl-targets=amdgcn-amd-amdhsa %S/Inputs/is_compatible_with_env.cpp -o %t.out

Expand Down
1 change: 0 additions & 1 deletion sycl/test-e2e/OptionalKernelFeatures/is_compatible/is_compatible_nvptx64.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// REQUIRES: cuda, opencl, gpu, cpu
// REQUIRES: build-and-run-mode

// RUN: %clangxx -fsycl -fsycl-targets=nvptx64-nvidia-cuda %S/Inputs/is_compatible_with_env.cpp -o %t.out

Expand Down
3 changes: 1 addition & 2 deletions sycl/test-e2e/OptionalKernelFeatures/is_compatible/is_compatible_several_targets.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
// REQUIRES: ocloc, any-device-is-level_zero, any-device-is-gpu, any-device-is-cpu
// REQUIRES: build-and-run-mode

// RUN: %clangxx -fsycl -fsycl-targets=spir64_gen -Xsycl-target-backend=spir64_gen "-device *" %S/Inputs/is_compatible_with_env.cpp -o %t.out

// RUN: env ONEAPI_DEVICE_SELECTOR=opencl:cpu %{run-unfiltered-devices} not %t.out
// RUN: env ONEAPI_DEVICE_SELECTOR=opencl:gpu %{run-unfiltered-devices} %t.out
// RUN: env ONEAPI_DEVICE_SELECTOR=level_zero:gpu %{run-unfiltered-devices} %t.out
// RUN: env ONEAPI_DEVICE_SELECTOR=level_zero:gpu %{run-unfiltered-devices} %t.out
1 change: 0 additions & 1 deletion sycl/test-e2e/OptionalKernelFeatures/is_compatible/is_compatible_spir64.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// REQUIRES: cuda, opencl, gpu, cpu
// REQUIRES: build-and-run-mode

// RUN: %clangxx -fsycl -fsycl-targets=spir64 %S/Inputs/is_compatible_with_env.cpp -o %t.out

Expand Down
1 change: 0 additions & 1 deletion sycl/test-e2e/OptionalKernelFeatures/is_compatible/is_compatible_spir64_fpga.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// REQUIRES: opencl-aot, accelerator, gpu, cpu
// REQUIRES: build-and-run-mode

// RUN: %clangxx -fsycl -fsycl-targets=spir64_fpga %S/Inputs/is_compatible_with_env.cpp -o %t.out

Expand Down
1 change: 0 additions & 1 deletion sycl/test-e2e/OptionalKernelFeatures/is_compatible/is_compatible_spir64_gen.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// REQUIRES: ocloc, gpu, level_zero, cpu
// REQUIRES: build-and-run-mode

// RUN: %clangxx -fsycl -fsycl-targets=spir64_gen -Xsycl-target-backend "-device *" %S/Inputs/is_compatible_with_env.cpp -o %t.out

Expand Down
1 change: 0 additions & 1 deletion sycl/test-e2e/OptionalKernelFeatures/is_compatible/is_compatible_spir64_x86_64.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// REQUIRES: opencl-aot, cpu, gpu, level_zero
// REQUIRES: build-and-run-mode

// RUN: %clangxx -fsycl -fsycl-targets=spir64_x86_64 %S/Inputs/is_compatible_with_env.cpp -o %t.out

Expand Down
1 change: 0 additions & 1 deletion sycl/test-e2e/Regression/compile_on_win_with_mdd.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// REQUIRES: windows
// REQUIRES: build-and-run-mode

// RUN: %clangxx --driver-mode=cl -fsycl /MDd -c %s -o %t.obj
// RUN: %clangxx --driver-mode=cl -fsycl %t.obj -Wno-unused-command-line-argument -o %t.out
Expand Down
Loading
Loading