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

xsmm - update for function name change #1464

Merged
merged 3 commits into from May 2, 2024
Merged

Conversation

jeremylt
Copy link
Member

@jeremylt jeremylt commented Feb 6, 2024

This branch fixes #1462

It turns out it was just a name change. We can merge once libXSMM releases their new version, or sooner if needed.

Copy link
Member

@jedbrown jedbrown left a comment

Choose a reason for hiding this comment

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

Is there a way to identify the version? It seems like only the patch version has changed, like libxsmm main is now at 1.17-3708, and I assume 1.18 is next? Is there a better way?

@jeremylt
Copy link
Member Author

jeremylt commented Feb 6, 2024

I think I contributed a macro to them like ours. I think we need to ask them if a release is coming soon and then we can add a macro to that tensor file that uses their version marco

@sebastiangrimberg
Copy link
Collaborator

Is there a way to identify the version? It seems like only the patch version has changed, like libxsmm main is now at 1.17-3708, and I assume 1.18 is next? Is there a better way?

I believe v2.0 is next and that's what they are preparing for. libxsmm.h defines LIBXSMM_VERSION (and LIBXSMM_VERSION_MAJOR which should allow us to check. This is populated from the version.txt file in the libxsmm repo, which currently is at 1.17-3708 but I think should be updated to 2.0 when released.

@jeremylt
Copy link
Member Author

jeremylt commented Feb 7, 2024

libXSMM should be making a new release in roughly 2-3 months time

backends/xsmm/ceed-xsmm.h Outdated Show resolved Hide resolved
@jeremylt
Copy link
Member Author

jeremylt commented May 2, 2024

The LIBXSMM release is taking a while. What about merging this fix now since we're already requiring a min version that isn't the latest release. Then we can switch the min version to the next LIBXSMM release when it happens

@sebastiangrimberg
Copy link
Collaborator

I agree with that. In Palace we are just applying the changes from this PR as a patch for all LIBXSMM builds (https://github.com/awslabs/palace/blob/main/extern/patch/libCEED/patch_xsmm.diff).

Copy link
Member

@jedbrown jedbrown left a comment

Choose a reason for hiding this comment

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

Okay, but we should document the min version somewhere more than CI, as this bleeding edge non-release version may be surprising to users.

@sebastiangrimberg
Copy link
Collaborator

Okay, but we should document the min version somewhere more than CI, as this bleeding edge non-release version may be surprising to users.

This is true. Spack builds will fail by default too unless the user builds with libxsmm@=main explicitly since the newest tagged version in the registry is also too old.

@jeremylt jeremylt merged commit f748815 into main May 2, 2024
28 checks passed
@jeremylt jeremylt deleted the jeremy/libxsmm-update branch May 2, 2024 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LIBXSMM interface broken with libxsmm/main
3 participants