Skip to content
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

[DirectX] Implement shader flag analysis for EnableRawAndStructuredBuffers #122663

Closed
Tracked by #88 ...
bogner opened this issue Jan 13, 2025 · 9 comments · Fixed by #122667
Closed
Tracked by #88 ...

[DirectX] Implement shader flag analysis for EnableRawAndStructuredBuffers #122663

bogner opened this issue Jan 13, 2025 · 9 comments · Fixed by #122667
Assignees

Comments

@bogner
Copy link
Contributor

bogner commented Jan 13, 2025

If Raw or Structured buffers are used, the EnableRawAndStructuredBuffers shader flag must be set. We can implement this by looking at the resource kind of the return type of llvm.dx.resource.handlefrom intrinsics.

@bogner
Copy link
Contributor Author

bogner commented Jan 13, 2025

Needed for particle_life

bogner added a commit to bogner/llvm-project that referenced this issue Jan 13, 2025
When raw or structured buffers are used, we need to set the DXIL flag
saying so.

Fixes llvm#122663.
@damyanp
Copy link
Contributor

damyanp commented Jan 13, 2025

@bharadwajy - is there an existing issue tracking this?

@bharadwajy
Copy link
Contributor

@bharadwajy - is there an existing issue tracking this?

No. It appears that I missed filing one earlier.

@damyanp
Copy link
Contributor

damyanp commented Jan 13, 2025

No. It appears that I missed filing one earlier.

@bharadwajy - is this a one off, or is this one of a class of flags that got missed?

@bharadwajy
Copy link
Contributor

No. It appears that I missed filing one earlier.

@bharadwajy - is this a one off, or is this one of a class of flags that got missed?

It is a one off and has been added to the class of flags based on resource properties as part of creation of this issue by @bogner. I've also added this to the issue that tracks implementation of flags needed for DML Shader compilation.

@damyanp
Copy link
Contributor

damyanp commented Jan 13, 2025

Ok - Justin seemed to suggest it was needed for particle_life?

@bharadwajy
Copy link
Contributor

bharadwajy commented Jan 13, 2025

Ok - Justin seemed to suggest it was needed for particle_life?

Looking at some history, my notes and recollecting earlier conversations, I did have an issue to track setting of EnableRawAndStructuredBuffers as needed by particle_life. This was closed as not needed with a link to the issue that is expected to address this.

@damyanp
Copy link
Contributor

damyanp commented Jan 13, 2025

Thanks, that looks like good context to have....someone should have a dig in to whether or not this means we still need this flag or not, I suppose that's step 1 for working on this issue.

@bogner
Copy link
Contributor Author

bogner commented Jan 14, 2025

We either need to implement this for particle_life, or we need microsoft/DirectXShaderCompiler#7003 to be done in dxc before we can make progress. We can't sign dxil that uses raw and structured buffers unless this flag is set today.

IMO we should re-open #112273, as it is not appropriate to close it unless we're committing to doing the work in microsoft/DirectXShaderCompiler#7003 immediately.

@davidcook-msft davidcook-msft moved this from Planning to Needs Review in HLSL Support Jan 14, 2025
bogner added a commit to bogner/llvm-project that referenced this issue Jan 20, 2025
When raw or structured buffers are used, we need to set the DXIL flag
saying so.

Fixes llvm#122663.
bogner added a commit that referenced this issue Jan 21, 2025
When raw or structured buffers are used, we need to set the DXIL flag
saying so.

Fixes #122663.
@github-project-automation github-project-automation bot moved this from Needs Review to Closed in HLSL Support Jan 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Closed
Development

Successfully merging a pull request may close this issue.

3 participants