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 missing deps in dev igc installation #13610

Merged
merged 10 commits into from
May 3, 2024
Merged

[CI] Fix missing deps in dev igc installation #13610

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
5019766
[CI] Fix missing deps in dev igc installation
jsji Apr 30, 2024
b2d317c
Fix copy/paste error
jsji Apr 30, 2024
0dcd4d2
Only test matrix for igc dev driver update
jsji May 1, 2024
b8803d5
Fix the order
jsji May 1, 2024
e9d5594
Add comments
jsji May 1, 2024
2c3f1b1
Test ESIMD as well when updating igc dev driver
jsji May 1, 2024
9e8c73e
Update igc dev
jsji May 2, 2024
90aa85b
Update igc dev to 41e87ac
jsji May 2, 2024
2a51a9a
Add workaround for hardware_dispatch
jsji May 2, 2024
4b16314
fix code formatting
jsji May 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .github/workflows/sycl-linux-precommit.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,9 @@ jobs:
- name: Determine Arc tests
id: arc_tests
run: |
if [ "${{ contains(needs.detect_changes.outputs.filters, 'drivers') }}" == "true" ]; then
if [ "${{ contains(needs.detect_changes.outputs.filters, 'devigccfg') }}" == "true" ]; then
echo 'arc_tests="Matrix/"' >> "$GITHUB_OUTPUT"
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd think we need to run all tests here as well.

Copy link
Contributor Author

@jsji jsji May 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

There are existing failures for non joint matrix tests.
eg: https://github.com/intel/llvm/actions/runs/8903576097/job/24452097484

We will only use igc dev driver for joint matrix for now, so we don't really need to run all e2e test,
only make sure joint matrix tests are clean is enough.

Running all tests would be an ideal solution, but to be honest, I don't think we have enough resource to triage and response to failures, especially in non joint matrix ones, so running all tests might effectively stop us from updating igc dev driver for joint matrix tests.

Copy link
Contributor

Choose a reason for hiding this comment

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

What about ESIMD team's tests? + @sarnex

Copy link
Contributor

Choose a reason for hiding this comment

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

TBH our tests take a long time to run. What is the use case of using the dev IGC driver? If I know that I can suggest if we need to run ESIMD tests with it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The dev IGC driver will be used in joint matrix pre-commit CI tests so that we can test some features that depends on newer IGC and get integrated test feedback earlier than before.

I would personally recommend that we also enable the pre-commit CI for ESIMD, because ESIMD features tend to depends on corresponding IGC changes as well. But it is up to your team to decide.

The cost of enabling ESIMD test here is that we may need to response to potential failures in CI , it would be great if the failures are real regressions that we need to resolve, but there will definitely be noise due to IGC changes etc.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks and yeah I agree ESIMD is very sensitive to IGC versions as well. I think we should enable dev IGC driver testing in ESIMD.

@v-klochkov FYI

elif [ "${{ contains(needs.detect_changes.outputs.filters, 'drivers') }}" == "true" ]; then
echo 'arc_tests=""' >> "$GITHUB_OUTPUT"
elif [ "${{ contains(needs.detect_changes.outputs.filters, 'esimd') }}" == "true" ]; then
echo 'arc_tests="(ESIMD|InvokeSimd|Matrix)/"' >> "$GITHUB_OUTPUT"
Expand Down
8 changes: 4 additions & 4 deletions devops/dependencies-igc-dev.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
{
"linux": {
"igc_dev": {
"github_tag": "igc-dev-498324a",
"version": "498324a",
"updated_at": "2024-04-17T13:44:27Z",
"url": "https://api.github.com/repos/intel/intel-graphics-compiler/actions/artifacts/1422168296/zip",
"github_tag": "igc-dev-d3f2b0e",
"version": "d3f2b0e",
"updated_at": "2024-04-30T21:45:41Z",
"url": "https://api.github.com/repos/intel/intel-graphics-compiler/actions/artifacts/1462213804/zip",
"root": "{DEPS_ROOT}/opencl/runtime/linux/oclgpu"
}
}
Expand Down
37 changes: 21 additions & 16 deletions devops/scripts/install_drivers.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,33 +108,38 @@ InstallIGFX () {
echo "Compute Runtime version $CR_TAG"
echo "CM compiler version $CM_TAG"
echo "Level Zero version $L0_TAG"
IS_IGC_DEV=$(CheckIGCdevTag $IGCTAG)
IGNORE_CHECKSUM=false
DPKG_OPTIONS=""
if [ "$IS_IGC_DEV" == "Yes" ]; then
echo "IGC dev git hash $IGC_DEV_VER"
get_pre_release_igfx $IGC_DEV_URL $IGC_DEV_VER
IGNORE_CHECKSUM=true
DPKG_OPTIONS=" --force-depends-version"
else
echo "IGC version $IGC_TAG"
get_release intel/intel-graphics-compiler $IGC_TAG \
| grep ".*deb" \
| wget -qi -
fi
echo "IGC version $IGC_TAG"
get_release intel/intel-graphics-compiler $IGC_TAG \
jsji marked this conversation as resolved.
Show resolved Hide resolved
| grep ".*deb" \
| wget -qi -
get_release intel/compute-runtime $CR_TAG \
| grep -E ".*((deb)|(sum))" \
| wget -qi -
# Perform the checksum conditionally and then get the release
( [ "$IGNORE_CHECKSUM" ] || sha256sum -c *.sum ) && \
sha256sum -c *.sum && \
get_release intel/cm-compiler $CM_TAG \
| grep ".*deb" \
| grep -v "u18" \
| wget -qi -
get_release oneapi-src/level-zero $L0_TAG \
| grep ".*deb" \
| wget -qi -
dpkg -i $DPKG_OPTIONS *.deb && rm *.deb *.sum
dpkg -i *.deb && rm *.deb *.sum
IS_IGC_DEV=$(CheckIGCdevTag $IGCTAG)
if [ "$IS_IGC_DEV" == "Yes" ]; then
# Dev IGC deb package did not include libopencl-clang
# So we need to backup and install it from release igc.
Copy link
Contributor

Choose a reason for hiding this comment

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

@bashbaug , do you know who can fix that on the IGC side?

Copy link
Contributor

Choose a reason for hiding this comment

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

I would suggest starting with @pszymich.

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 @aelovikov-intel and @bashbaug ! I had a quick look at https://github.com/intel/intel-graphics-compiler/blob/b077b43fa6193c97bcd2f3959f6d11e185d6fef3/.github/workflows/build-IGC.yml#L39C1-L41C56 , looks like IGC is also doing the hacky install/copy/package due to the fact that there is no libopencl-clang deb available.

I have contacted the libopencl-clang team to see whether we can build/release packaged libopencl-clang deb file in https://github.com/intel/opencl-clang/releases too. If that is fixed, then we should be able to remove this workaround, and also the workaround in IGC CI.

echo "Backup libopencl-clang"
cp -d /usr/local/lib/libopencl-clang.so.14* .
echo "Download IGC dev git hash $IGC_DEV_VER"
get_pre_release_igfx $IGC_DEV_URL $IGC_DEV_VER
echo "Install IGC dev git hash $IGC_DEV_VER"
dpkg -i *.deb
echo "Install libopencl-clang"
cp -d libopencl-clang.so.14* /usr/local/lib/
echo "Clean up"
rm *.deb libopencl-clang.so.14*
fi
}

InstallCPURT () {
Expand Down
Loading