Skip to content

[Driver][SYCL] Generalize huge device code driver option name #9888

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Jun 22, 2023
Merged

[Driver][SYCL] Generalize huge device code driver option name #9888

merged 6 commits into from
Jun 22, 2023

Conversation

tcreech-intel
Copy link
Contributor

Deprecate -fsycl-link-huge-device-code in favor of a new option,
-flink-huge-device-code. The new option is identical in functionality
but allowed with -fopenmp-targets.

This also includes a minor improvement to deprecated handling in order
to allow aliasing driver options to be deprecated independently.

@tcreech-intel
[Driver][SYCL] Generalize huge device code linking support
a041bda 
Deprecate -fsycl-link-huge-device-code in favor of a new option,
-flink-huge-device-code. The new option is identical in functionality
but allowed with -fopenmp-targets.

This also includes a minor improvement to deprecated handling in order
to allow aliasing driver options to be deprecated independently.
@tcreech-intel tcreech-intel changed the title [Driver][SYCL] Generalize huge device code linking support [Driver][SYCL] Generalize huge device code driver option name Jun 14, 2023
@tcreech-intel tcreech-intel temporarily deployed to aws June 14, 2023 20:59 — with GitHub Actions Inactive
@tcreech-intel tcreech-intel temporarily deployed to aws June 14, 2023 21:37 — with GitHub Actions Inactive
@tcreech-intel tcreech-intel marked this pull request as ready for review June 14, 2023 22:02
@tcreech-intel tcreech-intel requested review from a team as code owners June 14, 2023 22:02
mdtoguchi
mdtoguchi reviewed Jun 14, 2023
View reviewed changes
clang/test/Driver/sycl-huge-device-code-linker-script.cpp Outdated
// CHECK-LINKER-SCRIPT: "-T" "{{.*}}.ld"

// Also check that a user-provided linker script may be used:
// RUN: %clangxx -### -fsycl -fsycl-link-huge-device-code %s \
// RUN: -T custom-user-script.ld 2>&1 | \
// RUN: FileCheck --check-prefixes=CHECK-USER-SCRIPT %s
// RUN: %clangxx -### -fopenmp -fopenmp-targets=x86_64 %s \
// RUN: -T custom-user-script.ld 2>&1 | \
// RUN: FileCheck --check-prefixes=CHECK-USER-SCRIPT %s
Copy link
Contributor

Choose a reason for hiding this comment

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

Test name is specific to SYCL behaviors - we could either create a new test for OpenMP behaviors or rename this test to something more fitting.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. I decided to rename this test after the new option, with a comment describing that the old option name which is also being tested is deprecated.

@tcreech-intel tcreech-intel temporarily deployed to aws June 15, 2023 14:56 — with GitHub Actions Inactive
mdtoguchi
mdtoguchi approved these changes Jun 15, 2023
View reviewed changes
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

@tcreech-intel tcreech-intel temporarily deployed to aws June 15, 2023 15:35 — with GitHub Actions Inactive
@tcreech-intel tcreech-intel temporarily deployed to aws June 15, 2023 19:07 — with GitHub Actions Inactive
@tcreech-intel tcreech-intel temporarily deployed to aws June 17, 2023 12:24 — with GitHub Actions Inactive
@tcreech-intel tcreech-intel temporarily deployed to aws June 21, 2023 02:38 — with GitHub Actions Inactive
@tcreech-intel tcreech-intel temporarily deployed to aws June 21, 2023 04:30 — with GitHub Actions Inactive
@tcreech-intel tcreech-intel temporarily deployed to aws June 21, 2023 12:26 — with GitHub Actions Inactive
@tcreech-intel
Merge remote-tracking branch 'origin/sycl' into huge_code_rename
2b4d946 
Merging in an attempt to restart testing after potential
infrastructure-related failures.
@tcreech-intel tcreech-intel temporarily deployed to aws June 21, 2023 19:09 — with GitHub Actions Inactive
@tcreech-intel tcreech-intel temporarily deployed to aws June 21, 2023 19:47 — with GitHub Actions Inactive
@tcreech-intel
Copy link
Contributor Author

I don't think that the failing test is related to the change made in this PR.

@tcreech-intel
Copy link
Contributor Author

@intel/dpcpp-doc-reviewers, could you please take a look?

@bader bader mentioned this pull request Jun 22, 2023
Merged
@steffenlarsen
Copy link
Contributor

HIP Failed Tests (1):
SYCL :: Basic/event_profiling_info.cpp
This was recently enabled and I think the order issue was unexpected. Tag @mmoadeli .

Merging this.

@steffenlarsen steffenlarsen merged commit 79ea9b7 into intel:sycl Jun 22, 2023
@mmoadeli
Copy link
Contributor

mmoadeli commented Jun 22, 2023 •
edited
Loading

HIP Failed Tests (1): SYCL :: Basic/event_profiling_info.cpp This was recently enabled and I think the order issue was unexpected. Tag @mmoadeli .

Merging this.

That's a bit odd. Different bug, though.
It's likely it happens because the second kernel does nothing and the End timestamp reaches ready earlier some times. I add a bit functionality to the second kernel and will run it for a while.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bader bader bader approved these changes

@mdtoguchi mdtoguchi mdtoguchi approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@tcreech-intel @steffenlarsen @mmoadeli @bader @mdtoguchi