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

Jump to bottom

[CI] Fix HIP postcommit #14980

Merged
merged 1 commit into from
Aug 7, 2024
Merged

[CI] Fix HIP postcommit #14980

merged 1 commit into from
Aug 7, 2024

Conversation

sarnex
Copy link
Contributor

@sarnex sarnex commented Aug 7, 2024 •
edited
Loading

We need reset_gpu to be false, to use the correct image, and to use a static library build becuase lld is not packaged correctly into the binary for shared library build.

I'll investigate the lld issue separately (#14986), but let's unbreak CI.

@sarnex
[CI] Fix HIP postcommit
f034a9e 
Signed-off-by: Sarnie, Nick <nick.sarnie@intel.com>
@sarnex
Copy link
Contributor Author

sarnex commented Aug 7, 2024 •
edited
Loading

HIP runs successfully and the single fail is very unlikely to be related:

2024-08-07T13:53:40.3734710Z ********************
2024-08-07T13:53:40.3734774Z Failed Tests (1):
2024-08-07T13:53:40.3734864Z   SYCL :: AmdNvidiaJIT/kernel_and_bundle.cpp
2024-08-07T13:53:40.3734867Z 
2024-08-07T13:53:40.3734870Z 
2024-08-07T13:53:40.3734941Z Testing Time: 664.38s
2024-08-07T13:53:40.3734944Z 
2024-08-07T13:53:40.3735016Z Total Discovered Tests: 2129
2024-08-07T13:53:40.3735088Z   Unsupported      : 1320 (62.00%)
2024-08-07T13:53:40.3735157Z   Passed           :  803 (37.72%)

@sarnex sarnex marked this pull request as ready for review August 7, 2024 14:43
@sarnex sarnex requested a review from a team as a code owner August 7, 2024 14:43
@sarnex
Copy link
Contributor Author

sarnex commented Aug 7, 2024 •
edited
Loading

@intel/dpcpp-devops-reviewers Would appreciate a quick review on this one to fix postcommit CI. It took some more changes than expected because I hit an existing packaging issue of the SYCL toolchain. Will fix that separately but I need to investigate it.

@bader
Copy link
Contributor

bader commented Aug 7, 2024

I'll investigate the lld issue separately (#14986), but let's unbreak CI.

Why do we use lld built from sources? Can we install one with OS package manager? My understanding that CI jobs running tests are supposed to use binaries from build directory.

@sarnex
Copy link
Contributor Author

sarnex commented Aug 7, 2024

@bader Sure, I can try that, but I would prefer to unbreak CI ASAP.

I can commit to investigating and applying that fix (moving to package manager based LLD) today.

Okay with you?

@bader
Copy link
Contributor

bader commented Aug 7, 2024

Okay.

@sarnex
Copy link
Contributor Author

sarnex commented Aug 7, 2024

Appreciate the quick reviews/feedback.

For some reasons postcommit runs twice but every job has passed at least once (besides HIP failure explained above), so merging now.

Will work on LLD thing now.

@sarnex sarnex merged commit c1a3fc9 into sycl Aug 7, 2024
31 of 33 checks passed
This was referenced Aug 7, 2024
Merged
Open
sarnex added a commit that referenced this pull request Aug 7, 2024
@sarnex
[CI][NFC] Rename reset_gpu variable (#14987)
ff06bac 
It only works for Intel GPUs. 

Based on feedback from
[here](#14980 (comment)).

Signed-off-by: Sarnie, Nick <nick.sarnie@intel.com>
@aelovikov-intel aelovikov-intel deleted the sycl-devops-pr/hipfix branch August 13, 2024 21:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bader bader bader left review comments

@uditagarwal97 uditagarwal97 uditagarwal97 approved these changes

@aelovikov-intel aelovikov-intel Awaiting requested review from aelovikov-intel

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sarnex @bader @uditagarwal97