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

[SER] Patch 1: HitObject type lowering and SM 6.9 enablement #7097

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

simoll
Copy link

@simoll simoll commented Jan 29, 2025

Reduction of the complete SER implementation to just the HitObject type and its default constructor.

This has most of the infrastructure changes in DXC to support SER, eg static member functions for builtins, HitObject scalar type.

Specification PR: microsoft/hlsl-specs#277

Reduction of the complete SER implementation to just the HitObject type
and its default constructor.

Specification PR: microsoft/hlsl-specs#277
@simoll simoll requested a review from a team as a code owner January 29, 2025 12:27
@simoll
Copy link
Author

simoll commented Jan 29, 2025

@microsoft-github-policy-service agree company="NVIDIA"

When specifying more than four stages in a PAQ, the
SmallVector in the PayloadAccessAnnotation starts to allocate memory.
That memory is never free'd.

The hitobject-entry-errors accidentally triggered this:

struct [raypayload] Payload
{
    float elem
-          : write(caller,closesthit,anyhit,closesthit,miss)
-          : read(caller,closesthit,anyhit,closesthit,miss);
+          : write(caller,anyhit,closesthit,miss)
+          : read(caller,anyhit,closesthit,miss);
};
@@ -613,6 +613,22 @@ bool IsHLSLResourceType(clang::QualType type) {
return false;
}

bool IsHLSLHitObjectType(QualType type) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

There's already a IsHLSLHitObjectTy in DxilUtil.cpp. Though I know this functions differently, I would think getting the underlying Type object within QualType, and then calling IsHLSLHitObjectTy on that, would return a sensible result. Because that isn't the implementation, would you be able to update the function names so that they're different?

Copy link
Author

Choose a reason for hiding this comment

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

The patch uses the same naming and implementation approach for HitObject as was done for NodeOutput and RayQuery. The name IsHLSLHitObjectType says what the function does not how it is implemented, so i don't find that argument very convincing.
Do we really want to change this?

tools/clang/lib/CodeGen/CGHLSLMSFinishCodeGen.cpp Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: New
Development

Successfully merging this pull request may close these issues.

5 participants