Skip to content

[SYCL][CUDA][HIP] Propagate -foffload-fp32-prec-sqrt #17044

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

Merged
merged 4 commits into from
Feb 20, 2025

Conversation

MrSidims
Copy link
Contributor

@MrSidims MrSidims commented Feb 18, 2025

-foffload-fp32-prec-sqrt and -fsycl-fp32-prec-sqrt options should be merged together as they have the same purpose.
In this patch ability of -fsycl-fp32-prec-sqrt to pass appropriate options to CUDA and HIP compilers was added to
-foffload-fp32-prec-sqrt to allow such merge in the future.

It follows the approach from intel#5141
and intel#5309 adding intermediate
fcuda-prec-div flag.

Signed-off-by: Sidorov, Dmitry <[email protected]>
!0 = !{i32 4, !"nvvm-reflect-ftz", i32 1}
!1 = !{i32 4, !"nvvm-reflect-prec-sqrt", i32 1}
Copy link
Contributor Author

@MrSidims MrSidims Feb 18, 2025

Choose a reason for hiding this comment

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

This line seem to be lost during pulldown (at least git log/blame doesn't show any patches explicitly removing it), so I'm restoring it here.

Signed-off-by: Sidorov, Dmitry <[email protected]>
@MrSidims MrSidims marked this pull request as ready for review February 18, 2025 13:49
@MrSidims MrSidims requested review from a team as code owners February 18, 2025 13:49
@MrSidims MrSidims changed the title [SYCL][NVPTX][HIP] Propagate -foffload-fp32-prec-div/sqrt [SYCL][CUDA][HIP] Propagate -foffload-fp32-prec-div/sqrt Feb 18, 2025
Signed-off-by: Sidorov, Dmitry <[email protected]>
@MrSidims MrSidims changed the title [SYCL][CUDA][HIP] Propagate -foffload-fp32-prec-div/sqrt [SYCL][CUDA][HIP] Propagate -foffload-fp32-prec-sqrt Feb 18, 2025
Signed-off-by: Sidorov, Dmitry <[email protected]>
@MrSidims
Copy link
Contributor Author

@intel/dpcpp-tools-reviewers @intel/dpcpp-clang-driver-reviewers please take a look

Copy link
Contributor

@mdtoguchi mdtoguchi left a comment

Choose a reason for hiding this comment

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

LGTM - if you could also update the description to give a quick overview of the usage against the existing -fsycl-fp32-prec-sqrt option.

Would it make sense to work towards deprecating -fsycl-fp32-prec-sqrt?

@MrSidims
Copy link
Contributor Author

MrSidims commented Feb 20, 2025

@mdtoguchi deprecation (or renaming of offload options) will be followed in #17033

also added description in the PR

@MrSidims MrSidims requested a review from a team February 20, 2025 11:00
@MrSidims
Copy link
Contributor Author

MrSidims commented Feb 20, 2025

@intel/llvm-gatekeepers please help with merge

not sure, if tools team approval is necessary here, the only file that slips to the 'default' owner is llvm/docs/NVPTXUsage.rst , but approval Nicolas in this regard is (IMHO) sufficient.

Copy link
Contributor

@vmaksimo vmaksimo left a comment

Choose a reason for hiding this comment

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

approve for doc file

@dm-vodopyanov dm-vodopyanov merged commit 46a6600 into intel:sycl Feb 20, 2025
30 checks passed
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.

5 participants