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

Clean up output Vertices and Indices types for mesh shader. #2804

Closed
csyonghe opened this issue Apr 14, 2023 · 1 comment · Fixed by #3702
Closed

Clean up output Vertices and Indices types for mesh shader. #2804

csyonghe opened this issue Apr 14, 2023 · 1 comment · Fixed by #3702
Assignees
Labels
goal:quality & productivity Quality issues and issues that impact our productivity coding day to day inside slang kind:cleanup tech debt and rough edges priority:medium nice to have in next milestone

Comments

@csyonghe
Copy link
Collaborator

In our mesh shader representation, we currently use out Vertices<T> and out Indices<T> type to represent output vertex and index arrays.

This representation isn't aligned with the semantics of out parameters. In this case, a Vertex<T> type represents a reference to a output buffer, and the reference itself shouldn't be an out. Instead, we should introduce OutputVertices<T> type and turn the parameter into just OutputVertices<T> type, similar to how OutputPatch type works.

The current representation caused an issue in the uninitialized out parameter checking pass (slang-ir-use-uninitialized-out-param.cppbecause we will generate code that firstloadthe reference from theout` parameter, and then write to the reference, which isn't correct in terms of types.

After we clean this up, we should be able to remove the temporary hack that skips checking these parameters in the slang-ir-use-uninitialized-out-param pass.

@natduca natduca added kind:cleanup tech debt and rough edges priority:medium nice to have in next milestone labels Dec 1, 2023
@natduca
Copy link

natduca commented Dec 1, 2023

From Yong, I came to understand that the situation here is that the shape we have now is based on compat with HLSL, which we want to keep. But ideally we need a parser stage solution that takes "hlsl dialect" input and lowers it into our internal "ideal" approach described above.

We would however, in that solution, maintain the external suport for hlsl's style. So we don't need to do this right way.

The reason we want to still do this is that we're misusing the type system, and that does cause us some problems long term. So we want to do the change.

With this said, language cleanup, nonbreaking once we do it, so Q3.

@natduca natduca added this to the Q3 2024 (Summer) milestone Dec 1, 2023
@swoods-nv swoods-nv added the goal:quality & productivity Quality issues and issues that impact our productivity coding day to day inside slang label Feb 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
goal:quality & productivity Quality issues and issues that impact our productivity coding day to day inside slang kind:cleanup tech debt and rough edges priority:medium nice to have in next milestone
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants