Skip to content

Conversation

@KFilipek
Copy link
Contributor

@KFilipek KFilipek commented Jan 9, 2025

Description

There is a problem with linkage under icx/icpx due to linking against compiler libs.
When executing tests without sourced paths to compiler (prevents from loading compiler version of UMF) there are errors of missing symbols/libs.

Considerations

There was 2 options:

  • link statically against compiler dependencies using -static-intel
  • omit linking Intel compiler extensions with -no-intel-lib (more info) or using another option -Wl,-as-needed'

First works like a charm, but the UMF contains icx/icpx needed dependencies compiled in, I don't know it's a problem or not.
Last option require to force gtest to compile the same way - without Intel compiler dependencies (e.g. svml, imf etc). According to gtest documentation we can share CMAKE_(C/CXX)_FLAGS. Another approach is to split steps and apply patch.

Fixes #843

Checklist

  • Code compiles without errors locally
  • All tests pass locally
  • CI workflows execute properly
  • CI workflows, not executed per PR (e.g. Nightly), execute properly
  • New tests added, especially if they will fail without my changes
  • Added/extended example(s) to cover this functionality
  • Extended the README/documentation
  • All newly added source files have a license
  • All newly added source files are referenced in CMake files
  • Logger (with debug/info/... messages) is used

@KFilipek KFilipek self-assigned this Jan 9, 2025
@KFilipek KFilipek force-pushed the test-icx branch 2 times, most recently from 6ed3b8e to c480959 Compare January 9, 2025 13:50
@lplewa lplewa changed the title Enable ICX again Make ICX enabled again Jan 9, 2025
@KFilipek KFilipek marked this pull request as ready for review January 9, 2025 15:11
@KFilipek KFilipek requested a review from a team as a code owner January 9, 2025 15:11
@KFilipek KFilipek requested review from PatKamin and ldorau January 10, 2025 10:03
Copy link
Contributor

@lukaszstolarczuk lukaszstolarczuk left a comment

Choose a reason for hiding this comment

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

heh, I appreciate using my branch for this PR, but it contains a test commit "[CI] test only icx" - I guess it should be removed ;-)

Copy link
Contributor

@lukaszstolarczuk lukaszstolarczuk left a comment

Choose a reason for hiding this comment

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

beside copyright and removing redundant commit - LGTM

@KFilipek
Copy link
Contributor Author

heh, I appreciate using my branch for this PR, but it contains a test commit "[CI] test only icx" - I guess it should be removed ;-)

I left it intentionally because it was also your work :P

@lukaszstolarczuk lukaszstolarczuk merged commit 9d98b90 into oneapi-src:main Jan 10, 2025
78 checks passed
@KFilipek KFilipek deleted the test-icx branch January 10, 2025 13:45
lukaszstolarczuk added a commit to lukaszstolarczuk/unified-memory-framework that referenced this pull request Feb 7, 2025
@lukaszstolarczuk lukaszstolarczuk mentioned this pull request Feb 7, 2025
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.

Re-enable ICX test in CI

5 participants