Skip to content

Update clang easyblock for newer LLVM/Clang versions #3754

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

Closed

Conversation

Flamefire
Copy link
Contributor

@Flamefire Flamefire commented Jun 3, 2025

(created using eb --new-pr)

Especially the OpenMP target/offloading handling has changed significantly in 19 and again in 20. Update the code and unify some parts with the LLVM easyblock to avoid code duplication and drift

@Flamefire Flamefire marked this pull request as draft June 3, 2025 09:30
Comment on lines -686 to +700
self.runtimes_cmake_args['LIBOMPTARGET_DEVICE_ARCHITECTURES'] = '%s' % '|'.join(gpu_archs)
self.runtimes_cmake_args['LIBOMPTARGET_DEVICE_ARCHITECTURES'] = '%s' % ';'.join(gpu_archs)
Copy link
Contributor

Choose a reason for hiding this comment

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

This actively breaks runtime builds, since ; is interpreted as the end of a CMake argument. We ran into this when fixing the LLVM EasyBlock for offloading, see:

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 haven't seen this is a argument to an argument. semicolons are list separators and we pass a list of key=value to a variable. So using the semicolon here makes it split the list inside a value instead of after it.

I haven't found any doc to support the use of the pipe but seems to be ok. But we can also just set this as a regular CMake variable and it will be passed through automatically

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah... the pipe isn't really documented, but there are a few places where LLVM does this internally. That's where I found this.

@Flamefire
Copy link
Contributor Author

Closing in favor of updating the LLVM easyblock

@Flamefire Flamefire closed this Jun 3, 2025
@Flamefire Flamefire deleted the 20250603113032_new_pr_clang branch June 3, 2025 12:11
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.

2 participants