Skip to content

Conversation

@jchlanda
Copy link
Contributor

Make sure that -mlong-double-64 option is supported on HIP/Cuda.

@jchlanda jchlanda requested a review from a team as a code owner December 20, 2024 14:20
@jchlanda
Copy link
Contributor Author

Fixes: #15852

@jchlanda
Copy link
Contributor Author

Looks like linux gen12 is experiencing some problems.

Copy link
Contributor

Choose a reason for hiding this comment

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

For all of these, I believe the expected option syntax is to use --target=x86_64-unknown-linux-gnu -fsycl-targets=nvptx64-nvidia-cuda. Use of --target=<triple> is preferred over -target <triple> moving forward.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. I've only updated tests that I've added in this patch.

As a side note, it seems strange that sycl target triple option gets only one hyphen, both seem to be following the same pattern of:

hyphen[hyphen]target_name=triple

but end up spelled out differently:

--target=<triple>

vs:

-fsycl-targets=<triple>

Copy link
Contributor

Choose a reason for hiding this comment

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

Use of --target=<triple> is a general option. There are some similar options for offloading that use the double hyphen syntax which are also general in nature for offloading. Consider -fsycl-targets=<target> to be in a similar usage family as -fopenmp-targets=<target>. Moving forward when looking at the new offloading model approach, use of --offload-arch=<arch> would be used to target other devices which better fits within the double hyphen syntax.

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.

Thanks, LGTM

@jchlanda jchlanda force-pushed the jakub/nvptx_long_double_64 branch from f29e426 to 83260bb Compare January 7, 2025 06:55
@jchlanda
Copy link
Contributor Author

jchlanda commented Jan 7, 2025

@intel/llvm-gatekeepers this should be ready to roll.

Thank you.

@sommerlukas sommerlukas merged commit 04f23ec into intel:sycl Jan 7, 2025
17 checks passed
@sommerlukas
Copy link
Contributor

The post-commit failure is unrelated to changes in this PR, the failure is already tracked in #8803.

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.

3 participants