Skip to content

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 <dmitry.sidorov@intel.com>
!llvm.module.flags = !{!0}
!llvm.module.flags = !{!0, !1, !2}
!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 <dmitry.sidorov@intel.com>
@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 <dmitry.sidorov@intel.com>
@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 <dmitry.sidorov@intel.com>
@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