-
Notifications
You must be signed in to change notification settings - Fork 734
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
[CI] Enable FPGA on postcommit for Linux #12673
Changes from 7 commits
4252f83
b17de98
87f717f
381e557
8b6a0aa
ad07e97
2b04122
afb59c4
29e31c6
3a5a1e0
487ef12
59e81bd
dbe8a96
04cbcce
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,10 @@ | ||
// UNSUPPORTED: hip | ||
// RUN: %{build} -fno-builtin -o %t.out | ||
// RUN: %{run} %t.out | ||
|
||
// TODO: Remove unsupported after fixing | ||
// https://github.com/intel/llvm/issues/12683 | ||
// UNSUPPORTED: accelerator | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. accelerator or fpga? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's accelerator only because that's what the name of the feature is (https://github.com/intel/llvm/blob/sycl/sycl/test-e2e/lit.cfg.py#L678) when fpga device is reported by sycl-ls. |
||
// | ||
// RUN: %{build} -fno-builtin -fsycl-device-lib-jit-link -o %t.out | ||
// RUN: %if !gpu %{ %{run} %t.out %} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,7 @@ | ||
// REQUIRES: gpu-intel-gen12 | ||
// TODO: Remove unsupported after fixing | ||
// https://github.com/intel/llvm/issues/12683 | ||
// UNSUPPORTED: accelerator | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does it mean we can remove "// UNSUPPORTED: accelerator" here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IIUC, this test failed on opencl:fpga with the following error. That's why I added "UNSUPPORTED: accelerator". I'm not sure why you are suggesting removing this?
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I thought your PR fixes #12683 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This test is supposed to be run on HW which doesn't support Joint Matrix and verifies that correct exception was generated. So, I would expect that UNSUPPORTED should not be required for accelerator. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see. So, should we add XFAIL for FPGA instead? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need @dm-vodopyanov to look at why it is failing as the author of the feature/test. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi @uditagarwal97, I don't fully understand, why the test even launched on opencl:fpga if we have @aelovikov-intel having
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's passed like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you @aelovikov-intel. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure |
||
|
||
// RUN: %{build} -o %t.out -DSYCL_EXT_ONEAPI_MATRIX_VERSION=4 | ||
// RUN: %{run} %t.out | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure, but it looks like we shouldn't turn off Assert/* tests on acc as it's some environment issue, see #12683 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you try to change
// RUN: %{run} %t.out &> %t.txt ; FileCheck %s --input-file %t.txt %if acc %{ --check-prefix=CHECK-ACC %}
to
// RUN: %{run} %t.out &> %t.txt ; FileCheck %s --input-file %t.txt %if accelerator %{ --check-prefix=CHECK-ACC %}
instead of adding
UNSUPPORTED
to Assert/* tests?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For some reason, '%if accelerator' didn't work. The assert tests are still failing because if wrong check prefix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it should be %fpga