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

[SYCL][Driver] Enable PCH inclusion while performing host compilation with -fsycl #9864

Merged
merged 14 commits into from
Jul 10, 2023

Conversation

srividya-sundaram
Copy link
Contributor

@srividya-sundaram srividya-sundaram commented Jun 14, 2023

This PR allows users to include a Pre-Compiled header (PCH) file while performing the host compilation step with -fsycl.

// Create PCH
clang++ -x c-header test.h

// Use PCH

clang++ -c -fsycl -include test.h test.cpp
clang++ -c -fsycl -include-pch test.h.gch test.cpp

@mdtoguchi
Copy link
Contributor

Any thoughts on consideration for allowing the use of PCH files with -fsycl, but restricting to only the host side?

@srividya-sundaram srividya-sundaram temporarily deployed to aws June 14, 2023 21:15 — with GitHub Actions Inactive
@srividya-sundaram srividya-sundaram temporarily deployed to aws June 14, 2023 21:54 — with GitHub Actions Inactive
@srividya-sundaram srividya-sundaram changed the title [Driver] Disable PCH inclusion in SYCL device mode. [WIP][Driver] Disable PCH inclusion in SYCL device mode. Jun 15, 2023
@srividya-sundaram srividya-sundaram temporarily deployed to aws June 15, 2023 21:21 — with GitHub Actions Inactive
@srividya-sundaram srividya-sundaram temporarily deployed to aws June 15, 2023 21:38 — with GitHub Actions Inactive
@srividya-sundaram srividya-sundaram changed the title [WIP][Driver] Disable PCH inclusion in SYCL device mode. [WIP][Driver] Enable PCH inclusion while performing host compilation with -fsycl Jun 16, 2023
@srividya-sundaram srividya-sundaram changed the title [WIP][Driver] Enable PCH inclusion while performing host compilation with -fsycl [SYCL][Driver] Enable PCH inclusion while performing host compilation with -fsycl Jun 16, 2023
@srividya-sundaram srividya-sundaram marked this pull request as ready for review June 16, 2023 17:36
@srividya-sundaram srividya-sundaram requested a review from a team as a code owner June 16, 2023 17:36
@srividya-sundaram srividya-sundaram temporarily deployed to aws June 16, 2023 17:53 — with GitHub Actions Inactive
@srividya-sundaram srividya-sundaram temporarily deployed to aws June 16, 2023 18:25 — with GitHub Actions Inactive
@srividya-sundaram srividya-sundaram temporarily deployed to aws June 20, 2023 04:06 — with GitHub Actions Inactive
@srividya-sundaram srividya-sundaram changed the title [SYCL][Driver] Enable PCH inclusion while performing host compilation with -fsycl [WIP][SYCL][Driver] Enable PCH inclusion while performing host compilation with -fsycl Jun 20, 2023
@srividya-sundaram srividya-sundaram changed the title [WIP][SYCL][Driver] Enable PCH inclusion while performing host compilation with -fsycl [SYCL][Driver] Enable PCH inclusion while performing host compilation with -fsycl Jun 21, 2023
@srividya-sundaram srividya-sundaram temporarily deployed to aws June 21, 2023 21:51 — with GitHub Actions Inactive
@srividya-sundaram srividya-sundaram temporarily deployed to aws June 21, 2023 22:32 — with GitHub Actions Inactive
@srividya-sundaram srividya-sundaram temporarily deployed to aws June 22, 2023 04:58 — with GitHub Actions Inactive
@srividya-sundaram srividya-sundaram temporarily deployed to aws June 22, 2023 05:37 — with GitHub Actions Inactive
@srividya-sundaram srividya-sundaram temporarily deployed to aws June 24, 2023 01:15 — with GitHub Actions Inactive
@srividya-sundaram srividya-sundaram temporarily deployed to aws June 24, 2023 01:55 — with GitHub Actions Inactive
@srividya-sundaram srividya-sundaram temporarily deployed to aws June 26, 2023 17:01 — with GitHub Actions Inactive
@srividya-sundaram srividya-sundaram temporarily deployed to aws June 26, 2023 17:58 — with GitHub Actions Inactive
@srividya-sundaram srividya-sundaram temporarily deployed to aws June 27, 2023 16:35 — with GitHub Actions Inactive
@srividya-sundaram srividya-sundaram temporarily deployed to aws June 27, 2023 17:21 — with GitHub Actions Inactive
@srividya-sundaram srividya-sundaram temporarily deployed to aws July 7, 2023 23:42 — with GitHub Actions Inactive
@bader
Copy link
Contributor

bader commented Jul 8, 2023

Any thoughts on consideration for allowing the use of PCH files with -fsycl, but restricting to only the host side?

What problems do we need to solve to enable PCH for device compilers?

@romanovvlad, FYI.

@srividya-sundaram srividya-sundaram temporarily deployed to aws July 8, 2023 00:23 — with GitHub Actions Inactive
@mdtoguchi
Copy link
Contributor

Any thoughts on consideration for allowing the use of PCH files with -fsycl, but restricting to only the host side?

What problems do we need to solve to enable PCH for device compilers?

@romanovvlad, FYI.

If the generation of the PCH files is OK for device, the issue is consumption. We could take it in a step in the direction of 'fat PCH' files. If we encounter a fat PCH input file, we would recognize the targets, unbundle and pass along the unbundled PCH files to the respective compilations (host/device). For any device targets that are not found, we would ignore for those targets. Pass along the host variant to the host compile.

@srividya-sundaram srividya-sundaram temporarily deployed to aws July 8, 2023 01:18 — with GitHub Actions Inactive
@romanovvlad
Copy link
Contributor

Any thoughts on consideration for allowing the use of PCH files with -fsycl, but restricting to only the host side?

What problems do we need to solve to enable PCH for device compilers?
@romanovvlad, FYI.

If the generation of the PCH files is OK for device, the issue is consumption. We could take it in a step in the direction of 'fat PCH' files. If we encounter a fat PCH input file, we would recognize the targets, unbundle and pass along the unbundled PCH files to the respective compilations (host/device). For any device targets that are not found, we would ignore for those targets. Pass along the host variant to the host compile.

I noticed that there is some problem with deserialization of SYCL builtins , see fix here: https://github.com/romanovvlad/llvm/pull/7/files#diff-e2fdcbb3b7d1e3d5aa680d864906f9c49605080caa556ca2500c72b9c7a182c4R1815
This is not production quality fix though.

@bader
Copy link
Contributor

bader commented Jul 10, 2023

Any thoughts on consideration for allowing the use of PCH files with -fsycl, but restricting to only the host side?

What problems do we need to solve to enable PCH for device compilers?
@romanovvlad, FYI.

If the generation of the PCH files is OK for device, the issue is consumption. We could take it in a step in the direction of 'fat PCH' files. If we encounter a fat PCH input file, we would recognize the targets, unbundle and pass along the unbundled PCH files to the respective compilations (host/device). For any device targets that are not found, we would ignore for those targets. Pass along the host variant to the host compile.

I noticed that there is some problem with deserialization of SYCL builtins , see fix here: https://github.com/romanovvlad/llvm/pull/7/files#diff-e2fdcbb3b7d1e3d5aa680d864906f9c49605080caa556ca2500c72b9c7a182c4R1815 This is not production quality fix though.

This won't resolve the problem Mike mentioned, but I think we need to make this fix anyway. @elizabethandrews, @premanandrao, FYI.

@srividya-sundaram
Copy link
Contributor Author

@bader Can you please merge the changes in this PR? Thanks!

@bader bader merged commit 7f12b28 into intel:sycl Jul 10, 2023
14 checks passed
@bader
Copy link
Contributor

bader commented Jul 10, 2023

@bader Can you please merge the changes in this PR? Thanks!

Yes, but it's better to ping @intel/llvm-gatekeepers team as described in the contribution guide.

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.

4 participants