Skip to content

[SYCL][Test E2E] Use %{build}/%{run} in Plugin tests #9331

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

Merged
merged 2 commits into from
May 9, 2023

Conversation

aelovikov-intel
Copy link
Contributor

No description provided.

@aelovikov-intel aelovikov-intel requested a review from a team as a code owner May 6, 2023 04:02
@aelovikov-intel aelovikov-intel temporarily deployed to aws May 6, 2023 05:18 — with GitHub Actions Inactive
@aelovikov-intel aelovikov-intel temporarily deployed to aws May 6, 2023 09:30 — with GitHub Actions Inactive
// RUN: %clangxx -fsycl -fsycl-targets=%sycl_triple %s -o %t.out
// RUN: env SYCL_PI_TRACE=2 %GPU_RUN_PLACEHOLDER %t.out %GPU_CHECK_PLACEHOLDER
// RUN: env SYCL_PI_TRACE=2 %CPU_RUN_PLACEHOLDER %t.out %CPU_CHECK_PLACEHOLDER
// UNSUPPORTED: accelerator
Copy link
Contributor

Choose a reason for hiding this comment

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

We support multiple // UNSUPPORTED: directives? ( As opposed to having to construct a boolean expression with hip_nvidia )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes

// RUN: %clangxx -fsycl -fsycl-targets=%sycl_triple %s -o %t.out %level_zero_options %threads_lib
// RUN: %BE_RUN_PLACEHOLDER %t.out
// RUN: %{build} -o %t.out %level_zero_options %threads_lib
// RUN: %{run} %t.out
Copy link
Contributor

Choose a reason for hiding this comment

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

Did BE_RUN_PLACEHOLDER somehow also pipe to FileCheck? In any case, we'll need to add that now, corect?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, we never ran FileCheck on that test. I think the author supposed we need (hence checks below), but enabling that should be a separate PR.

// RUN: env ONEAPI_DEVICE_SELECTOR="level_zero:*" ZE_DEBUG=4 %GPU_RUN_PLACEHOLDER %t.out wait 2>&1 %GPU_CHECK_PLACEHOLDER
// RUN: env ONEAPI_DEVICE_SELECTOR="level_zero:*" ZE_DEBUG=4 %GPU_RUN_PLACEHOLDER %t.out nowait 2>&1 %GPU_CHECK_PLACEHOLDER
// RUN: %{build} %level_zero_options -o %t.out
// RUN: env ONEAPI_DEVICE_SELECTOR="level_zero:*" ZE_DEBUG=4 %{run} %t.out wait 2>&1 | FileCheck %s
Copy link
Contributor

@cperkinsintel cperkinsintel May 8, 2023

Choose a reason for hiding this comment

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

Since we have the //REQUIRES: level_zero already in the test, do we really need the ONEAPI_DEVICE_SELECTOR="level_zero:*" ?

The other tests have foregone it and I think they are improved for the change. Probably should remove it everyhwere in this test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it should be removed.

@@ -1,21 +1,17 @@
// REQUIRES: aspect-ext_intel_device_id
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably needs level_zero added to the // REQUIRES: directive

Copy link
Contributor

Choose a reason for hiding this comment

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

Someone put it at the bottom of the test. Guess their backspace key was broken.

// RUN: %clangxx -fsycl -fsycl-targets=%sycl_triple %level_zero_options %s -o %t.out
// RUN: env ONEAPI_DEVICE_SELECTOR="level_zero:*" ZE_DEBUG=1 %GPU_RUN_PLACEHOLDER %t.out 2>&1 %GPU_CHECK_PLACEHOLDER
// RUN: %{build} %level_zero_options -o %t.out
// RUN: env ONEAPI_DEVICE_SELECTOR="level_zero:*" ZE_DEBUG=1 %{run} %t.out 2>&1 | FileCheck %s
Copy link
Contributor

Choose a reason for hiding this comment

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

Again, let's remove ONEAPI_DEVICE_SELECTOR

// RUN: env SYCL_PI_LEVEL_ZERO_BATCH_SIZE=1 ONEAPI_DEVICE_SELECTOR="level_zero:*" %GPU_RUN_PLACEHOLDER %t.out
// RUN: env SYCL_PI_LEVEL_ZERO_BATCH_SIZE=2 ONEAPI_DEVICE_SELECTOR="level_zero:*" %GPU_RUN_PLACEHOLDER %t.out
// RUN: env SYCL_PI_LEVEL_ZERO_BATCH_SIZE=3 ONEAPI_DEVICE_SELECTOR="level_zero:*" %GPU_RUN_PLACEHOLDER %t.out
// RUN: env SYCL_PI_LEVEL_ZERO_BATCH_SIZE=0 ONEAPI_DEVICE_SELECTOR="level_zero:*" %{run} %t.out
Copy link
Contributor

Choose a reason for hiding this comment

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

remove ONEAPI_DEVICE_SELECTOR

// RUN: env SYCL_PI_LEVEL_ZERO_BATCH_SIZE=1 ONEAPI_DEVICE_SELECTOR="level_zero:*" %GPU_RUN_PLACEHOLDER %t.out
// RUN: env SYCL_PI_LEVEL_ZERO_BATCH_SIZE=2 ONEAPI_DEVICE_SELECTOR="level_zero:*" %GPU_RUN_PLACEHOLDER %t.out
// RUN: env SYCL_PI_LEVEL_ZERO_BATCH_SIZE=3 ONEAPI_DEVICE_SELECTOR="level_zero:*" %GPU_RUN_PLACEHOLDER %t.out
// RUN: env SYCL_PI_LEVEL_ZERO_BATCH_SIZE=0 ONEAPI_DEVICE_SELECTOR="level_zero:*" %{run} %t.out
Copy link
Contributor

Choose a reason for hiding this comment

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

ibid. ( remove ONEAPI_DEVICE_SELECTOR )

// RUN: %clangxx -fsycl -fsycl-targets=%sycl_triple %level_zero_options %threads_lib %s -o %t.out
// RUN: env SYCL_PI_LEVEL_ZERO_TRACK_INDIRECT_ACCESS_MEMORY=1 ONEAPI_DEVICE_SELECTOR="level_zero:*" %GPU_RUN_PLACEHOLDER %t.out 2>&1 %GPU_CHECK_PLACEHOLDER
// RUN: %{build} %level_zero_options %threads_lib -o %t.out
// RUN: env SYCL_PI_LEVEL_ZERO_TRACK_INDIRECT_ACCESS_MEMORY=1 ONEAPI_DEVICE_SELECTOR="level_zero:*" %{run} %t.out 2>&1 | FileCheck %s
Copy link
Contributor

Choose a reason for hiding this comment

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

probably can remove ONEAPI_DEVICE_SELECTOR=level_zero:*

Copy link
Contributor

@cperkinsintel cperkinsintel left a comment

Choose a reason for hiding this comment

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

sweet

@aelovikov-intel aelovikov-intel temporarily deployed to aws May 8, 2023 23:56 — with GitHub Actions Inactive
@aelovikov-intel aelovikov-intel temporarily deployed to aws May 9, 2023 02:11 — with GitHub Actions Inactive
@aelovikov-intel aelovikov-intel temporarily deployed to aws May 9, 2023 03:58 — with GitHub Actions Inactive
@aelovikov-intel aelovikov-intel temporarily deployed to aws May 9, 2023 05:13 — with GitHub Actions Inactive
@aelovikov-intel
Copy link
Contributor Author

Unexpectedly Passed Tests (1):
  SYCL :: KernelFusion/device_info_descriptor.cpp

is failing on other PRs as well. Reported in #8747 (comment).

@aelovikov-intel aelovikov-intel merged commit fa89b05 into intel:sycl May 9, 2023
@aelovikov-intel aelovikov-intel deleted the e2e-run-plugin branch May 9, 2023 05:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants