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

[CI][HIP] Add missing dep for broken amd script. #13619

Merged
merged 15 commits into from
May 9, 2024

Conversation

JackAKirk
Copy link
Contributor

@JackAKirk JackAKirk commented May 1, 2024

Fixes #13612

#13612 was caused by these issues:

ROCm/ROCm#2949
ROCm/HIP#3406

To fix this gcc/g++12 is installed manually and set as the preferred version.

Signed-off-by: JackAKirk <jack.kirk@codeplay.com>
Signed-off-by: JackAKirk <jack.kirk@codeplay.com>
Comment on lines 27 to 28
RUN ln -s -f /usr/bin/gcc-12 /usr/bin/gcc && \
ln -s -f /usr/bin/g++-12 /usr/bin/g++
Copy link
Contributor

Choose a reason for hiding this comment

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

I think proper way would be something using update-alternatives.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I've updated this now.

I've tested the latest commit now builds using g++12 for hip backend within a docker locally. Assuming that there is no backend that is incompatible with being built by g++12, this should now be safe.

Switch back to 12.1 (12.4 can be safely updated later).

Signed-off-by: JackAKirk <jack.kirk@codeplay.com>
@JackAKirk JackAKirk marked this pull request as ready for review May 6, 2024 17:14
@JackAKirk JackAKirk requested a review from a team as a code owner May 6, 2024 17:14
@aelovikov-intel
Copy link
Contributor

I think this needs to be split in two parts. First, fix the images but keep pre/post-commit running on old containers. Once merged and new images are verified locally we can switch the pre/post-commit tasks to use latest images once again.

Signed-off-by: JackAKirk <jack.kirk@codeplay.com>
@JackAKirk
Copy link
Contributor Author

I think this needs to be split in two parts. First, fix the images but keep pre/post-commit running on old containers. Once merged and new images are verified locally we can switch the pre/post-commit tasks to use latest images once again.

Makes sense to me. I've made this change.

@npmiller
Copy link
Contributor

npmiller commented May 9, 2024

@intel/llvm-gatekeepers this is ready to merge

Note that the containers for pre/post commit are currently fixed to an older version, so this will re-generate the latest but cannot break testing.

@martygrant martygrant merged commit 11ad4ec into intel:sycl May 9, 2024
6 checks passed
aelovikov-intel pushed a commit that referenced this pull request May 10, 2024
This partially reverts the changes made in
#13619 to use libstdc++-12-dev without
gcc12.
This also seems to work fine to build dpc++ within the docker. But
should come without the CI failures that might have been induced by the
gcc uplift:
- #13736
- #13735
If the above issues were not caused by the gcc version uplift, then this
patch should also reveal that.

Signed-off-by: JackAKirk <jack.kirk@codeplay.com>
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.

SYCL Pre Commit on Linux / test (AMD/HIP, ["Linux", "amdgpu"] - fails
4 participants