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

[SYCL] Fix failing specialization constants tests. #12839

Merged
merged 2 commits into from
Feb 28, 2024

Conversation

maarquitos14
Copy link
Contributor

Recent changes made to specialization constants tests relied on --debug-only option, which is only enabled when assertions are enabled. This commit guards the commands using such an option to be run only when assertions are enabled.

This should fix the post-commit CI failure observed after #12647 was merged.

Recent changes made to specialization constants tests relied on --debug-only option, which is only enabled when assertions are enabled. This commit guards the commands using such an option to be run only when assertions are enabled.

Signed-off-by: Marcos Maronas <marcos.maronas@intel.com>
@@ -1,6 +1,6 @@
; RUN: sycl-post-link -spec-const=emulation < %s -o %t.files.table
; RUN: FileCheck %s -input-file=%t.files_0.prop
; RUN: sycl-post-link -debug-only=SpecConst -spec-const=emulation < %s 2>&1 | FileCheck %s --check-prefix=CHECK-LOG
; RUN: %if asserts %{ sycl-post-link -debug-only=SpecConst -spec-const=emulation < %s 2>&1 | FileCheck %s --check-prefix=CHECK-LOG %} %else %{ %}
Copy link
Contributor

Choose a reason for hiding this comment

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

Empty else isn't necessary.

@aelovikov-intel
Copy link
Contributor

Should be up to the @intel/dpcpp-tools-reviewers if they're ok with reduced coverage (not performing checks in release builds in testing).

@maarquitos14
Copy link
Contributor Author

Should be up to the @intel/dpcpp-tools-reviewers if they're ok with reduced coverage (not performing checks in release builds in testing).

Actually, what was added in #12647 was extra coverage that required --debug-only, so I added new RUN lines for every test with the mentioned option. Conditioning the new RUN lines is not reducing the coverage any further than it was before #12647.

Signed-off-by: Marcos Maronas <marcos.maronas@intel.com>
Copy link
Contributor

@maksimsab maksimsab left a comment

Choose a reason for hiding this comment

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

LGTM

@steffenlarsen steffenlarsen merged commit 652f1c6 into intel:sycl Feb 28, 2024
12 checks passed
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.

5 participants