Skip to content

[SYCL][AMD] Propagate metadata in createURProgram #14831

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 5 commits into from
Aug 1, 2024

Conversation

Naghasan
Copy link
Contributor

@Naghasan Naghasan commented Jul 29, 2024

SYCL properties weren't converted when calling creatreURProgram, leading to issue in finalization during KernelFusion for AMD.

Fixes #14841

SYCL properties weren't converted when calling creatreURProgram,
leading to issue in finalization during KernelFusion for AMD.
@aelovikov-intel aelovikov-intel changed the title [SYCL][AMD] Propagate metadata in createURPRogram [SYCL][AMD] Propagate metadata in createURProgram Jul 29, 2024
@Naghasan
Copy link
Contributor Author

address a regression introduced with moving to UR #14598

@aelovikov-intel
Copy link
Contributor

@Naghasan
Copy link
Contributor Author

Naghasan commented Jul 29, 2024

address a regression introduced with moving to UR #14598

What I meant is https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword

That wasn't an answer to your comment. But doesn't apply here as the issue is tracking multiple issues, only address one aspect.

@sommerlukas
Copy link
Contributor

address a regression introduced with moving to UR #14598

What I meant is https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword

#14598 captures multiple issues that were detected during the PI removal. Not all of them are addressed by this PR, so I don't think it makes sense to use a keyword such as closes.

@sommerlukas
Copy link
Contributor

Just remembered that this PR should also reactivate this test: https://github.com/intel/llvm/blob/sycl/sycl/test-e2e/NewOffloadDriver/diamond_shape.cpp

@Naghasan Naghasan requested a review from a team as a code owner July 30, 2024 15:13
@Naghasan
Copy link
Contributor Author

Windows job failing for an unrelated reason

Post job cleanup.
Cleaning D:\github\_work\llvm\llvm/*
rm: cannot remove 'D:\github\_work\llvm\llvm/build-ee/ESIMD': Device or resource busy

fyi @intel/dpcpp-devops-reviewers

@sarnex
Copy link
Contributor

sarnex commented Jul 30, 2024

I restarted the runner ~1hr ago, should be fixed (tm). I'll restart the job here.

@callumfare
Copy link
Contributor

The DeviceGlobal tests that were recently disabled are fixed by this change and can be reenabled:

DeviceGlobal/device_global_arrow.cpp
DeviceGlobal/device_global_device_only.cpp
DeviceGlobal/device_global_operator_passthrough.cpp
DeviceGlobal/device_global_subscript.cpp

@Naghasan
Copy link
Contributor Author

reenabled

@Naghasan
Copy link
Contributor Author

ping @dpcpp-tools-reviewers

@Naghasan
Copy link
Contributor Author

@intel/llvm-gatekeepers the PR is ready to be merged

@sommerlukas sommerlukas merged commit 41d8977 into intel:sycl Aug 1, 2024
15 checks passed
@Naghasan Naghasan deleted the victor/fix-amd-kf branch August 1, 2024 07:43
AlexeySachkov pushed a commit to AlexeySachkov/llvm that referenced this pull request Nov 26, 2024
SYCL properties weren't converted when calling creatreURProgram, leading
to issue in finalization during KernelFusion for AMD.

Fixes intel#14841
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Basic/reqd_work_group_size.cpp
8 participants