Skip to content

[SYCL][NewOffloadModel] Set image compile/link options for non-SPIR targets#22748

Open
sarnex wants to merge 1 commit into
intel:syclfrom
sarnex:weird
Open

[SYCL][NewOffloadModel] Set image compile/link options for non-SPIR targets#22748
sarnex wants to merge 1 commit into
intel:syclfrom
sarnex:weird

Conversation

@sarnex

@sarnex sarnex commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

This one is pretty weird.

We have a test that makes sure if we compile with -g that we pass -g at runtime to UR.
The weird part is that they expect this for NVPTX, as well as SPIR.

In the old offload model we propagate -g to the image flags even for NVPTX, so do the same for the new offload model.

It is not totally clear if:

  1. The test intended to check this for NVPTX
  2. The old offload model intended to do this for NVPTX

The code that adds this for the old offload model is here, you can see the triple check at the link but the flag is actually added in AddSPIRVImpliedTargetArgs

The change is low impact and trivial so IMO let's just match support for the old offload model and move on.

@sarnex sarnex added the new-offload-model Enables testing with NewOffloadModel. label Jul 23, 2026
…argets

Signed-off-by: Nick Sarnie <nick.sarnie@intel.com>
@sarnex
sarnex marked this pull request as ready for review July 24, 2026 18:35
@sarnex
sarnex requested review from a team as code owners July 24, 2026 18:35

@mdtoguchi mdtoguchi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In regards to matching with what we did for the old-model, this makes sense. As you stated, it's not clear what the actual intentional behavior is. I would consider this as a side impact from initial early spir64 implementation and when enabling for 3rd party support, the behavior just took a ride with what was there.

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

Labels

new-offload-model Enables testing with NewOffloadModel.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants