Skip to content
This repository was archived by the owner on Aug 11, 2023. It is now read-only.

Various CMake fixes #291

Merged
merged 9 commits into from
Aug 11, 2023
Merged

Conversation

MathiasMagnus
Copy link
Contributor

Given Stream HPC's involvement with the SYCL CTS, we came across several shortcomings of the CMake support of ComputeCpp that are either breaking or just could be improved for better UX. The git commit messages should be descriptive enough.

@DuncanMcBain
Copy link
Member

I'm sorry I haven't managed to get to this yet, I will likely have to look at it in the new year as I'll be on holiday soon. I haven't forgotten though!

cmake_policy is set that all policies up to 3.24 are set to NEW.
CMP0116 from CMake 3.20 causes contents of depfiles to be updated
by CMake relative to a different directory
@MathiasMagnus
Copy link
Contributor Author

@DuncanMcBain I added another fix on top regarding the contents of depfiles generated by ComputeCpp when using CMake 3.20+. (It causes ninja to consider the ih target to be always out-of-date and forces a total rebuild. Quite annoying, given how long some CTS targets build.)

@DuncanMcBain
Copy link
Member

Thanks! So I have had a brief look at this, and one of the things I've not worked out how to resolve yet is that there are other FindComputeCpp changes floating around that I was meaning to add to this repository. I should have done that long before now and I'm paying the price. (This is also why I didn't want copies of the file to proliferate, they get out of sync and it's just a mess.)

I've not decided quite what to do. The changes to FindComputeCpp were added to SYCL-DNN some time last year. I had a work-in-progress patch to add them to this repository. They definitely conflict with what you have here, but it's probably most fair to you for me to review this, then to resolve the changes with our stuff.

@MathiasMagnus
Copy link
Contributor Author

I'd mostly want to see it work. Keeping attribution is a nice thing, but I won't hold it against anyone if it falls victim. Afterall, more was lost at Mohács.

@MathiasMagnus
Copy link
Contributor Author

(This is also why I didn't want copies of the file to proliferate, they get out of sync and it's just a mess.)

I wholeheartedly agree. I understand that the Find Module was spun out to the SDK so that it can be updated more frequently than the compiler releases. One such paraphrase of the scripts here were the ones in the CTS, and they were insanely out of date. Because the contents of the scripts here aligned with the Package Config file in recent compiler releases, I figured this is the "one source of truth". I didn't know about "rogue" updates laying around.

Anyhow, I'll let you figure out the best path forward.

@ProGTX
Copy link
Contributor

ProGTX commented Feb 2, 2023

We prefer to use the package config now, but this is still the place where we accept contributions, so thank you very much for this :)

@DuncanMcBain DuncanMcBain merged commit fb9b7ba into codeplaysoftware:master Aug 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants