Skip to content

Conversation

@mdtoguchi
Copy link
Contributor

When compiling for -fsycl-targets values of nvptx64-nvidia-cuda and amdgcn-amd-gpu, the default arch behaviors were not applied to the compilation. Updates to do the following:

  • Add default of sm_50 for nvptx64 if not provided
  • Emit diagnostic if no arch provided for amd
  • Parse -Xsycl-backend-target for offload-arch values

When compiling for -fsycl-targets values of nvptx64-nvidia-cuda and
amdgcn-amd-gpu, the default arch behaviors were not applied to the
compilation.  Updates to do the following:
 - Add default of sm_50 for nvptx64 if not provided
 - Emit diagnostic if no arch provided for amd
 - Parse -Xsycl-backend-target for offload-arch values
@mdtoguchi mdtoguchi requested a review from a team as a code owner June 28, 2024 00:55
@mdtoguchi mdtoguchi requested a review from asudarsa June 28, 2024 00:55
Copy link
Contributor

@asudarsa asudarsa left a comment

Choose a reason for hiding this comment

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

I tested the changes for the new offloading model on several failing SYCL E2E tests on an AMD machine and the fix resolves all of those failures. Test added looks fine.

Thanks much for the fix.

@mdtoguchi mdtoguchi requested a review from hchilama June 28, 2024 15:42
@mdtoguchi
Copy link
Contributor Author

@intel/llvm-gatekeepers, This should be ready for merge - please take a look.

@asudarsa
Copy link
Contributor

We should really be not running CI testing for comment updates...:-(

@martygrant
Copy link
Contributor

@mdtoguchi this is awaiting approval from @srividya-sundaram

@asudarsa
Copy link
Contributor

asudarsa commented Jul 1, 2024

We have approval from a driver team reviewer, This change unblocks development by fixing a large number of test failures. So, I ill merge it now. @srividya-sundaram, please feel free to reach out to me or @mdtoguchi for any post-commit changes if required.

Thanks
Sincerely

@asudarsa asudarsa merged commit c99522b into intel:sycl Jul 1, 2024
@srividya-sundaram
Copy link
Contributor

Sorry, I missed the notification for approval request.

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