Skip to content

Conversation

@simoll
Copy link
Collaborator

@simoll simoll commented Oct 22, 2025

  • DXIL validation tests showing correct uses pass

SM6.10 tracking bug: #7824

@V-FEXrt
Copy link
Collaborator

V-FEXrt commented Oct 24, 2025

Hey Simon!

We've actually been talking about DXIL op stuff internally that this PR is going to fall under. Specially we want a better system to mark ops as experimental before they are upgraded to stable!

I have a proposal up (microsoft/hlsl-specs#698) but due to scheduling conflicts we aren't going to be able to make a final decision until ~Nov 4th.

Since the decision there has impacts on this PR we are holding off on the review for just a little bit. I wanted to give you a heads up so you weren't surprised by the silence from our side

@V-FEXrt
Copy link
Collaborator

V-FEXrt commented Nov 12, 2025

microsoft/hlsl-specs#729

The proposal is up here, once the implementation lands the ops here will need to be renumbered. I suspect the implementation will land quite quickly

@V-FEXrt
Copy link
Collaborator

V-FEXrt commented Nov 14, 2025

Spec has been merged! I'm on vacation the next few weeks but @tex3d should be uploading the infrastructure change soon. Then this can be rebased on that

@simoll
Copy link
Collaborator Author

simoll commented Dec 1, 2025

Spec has been merged! I'm on vacation the next few weeks but @tex3d should be uploading the infrastructure change soon. Then this can be rebased on that

@tex3d What's the status of the experimental opcode infra change to unblock this PR?

@tex3d
Copy link
Contributor

tex3d commented Dec 1, 2025

@tex3d What's the status of the experimental opcode infra change to unblock this PR?

I'm trying to get a second review on this: #7947. Once merged, you can update the branch and move the intrinsic definitions in hctdb.py into experimental and update any references to the opcode numbers that changed.

Copy link
Contributor

@tex3d tex3d left a comment

Choose a reason for hiding this comment

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

Now that #7947 has merged, you'll need to rebase and move things to experimental ops according to the comments I made.

@tex3d
Copy link
Contributor

tex3d commented Dec 8, 2025

I believe we'll also want to reserve high-level intrinsic IDs for these ASAP. It might be good to do this in the same PR, but a separate PR for that would be ok too.

That means updates to gen_intrin_main.txt, generated files, as well as manually adding EmptyLower entries for gLowerTable in HLOperationLower.cpp for each new high-level intrinsic operation.

This could serve as a good example for how to reserve HLSL intrinsics and DXIL opcodes for a feature before it's implemented, using the new experimental DXIL op table.

@tex3d
Copy link
Contributor

tex3d commented Dec 10, 2025

I've created a PR to reserve HL and DXIL ops for these operations here: #7995.

So, you should be able to re-apply just the changes necessary on top of main when that PR merges.

@simoll
Copy link
Collaborator Author

simoll commented Dec 10, 2025

Thanks @tex3d. I'll rebase when #7995 lands

@simoll simoll changed the title [SM6.10] DXIL opcodes for ClusterID and TriangleObjectPositions [SM6.10] VEC9 TriangleObjectPositions / CHECK-pass validation tests Dec 15, 2025
@simoll simoll force-pushed the sm610_dxil_opcodes branch from b3a793f to 63a5b8b Compare December 15, 2025 05:59
@simoll simoll requested a review from tex3d December 15, 2025 06:01

; Function Attrs: nounwind
define void @"\01?test_closesthit@@YAXUPayload@@UBuiltInTriangleIntersectionAttributes@@@Z"(%struct.Payload* noalias nocapture %payload, %struct.BuiltInTriangleIntersectionAttributes* nocapture readnone %attr) #0 {
%1 = call i32 @dx.op.clusterID(i32 2147483651) ; ClusterID()
Copy link
Contributor

Choose a reason for hiding this comment

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

When this is round-tripped through llvm, i32 2147483651 will become i32 -2147483645. In lowering tests, for example, CHECK lines will need to use i32 -2147483645 instead of the unsigned value.

Using the unsigned value here could create confusion and additional difficulty for updating tests when moving ops to final DXIL because one will need to use two search/replace expressions to update each opcode, in case the unsigned form is used somewhere.

For this reason, I recommend considering the signed i32 form the canonical one to use in all cases for consistency.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
%1 = call i32 @dx.op.clusterID(i32 2147483651) ; ClusterID()
%1 = call i32 @dx.op.clusterID(i32 -2147483645) ; ClusterID()

Copy link
Contributor

@tex3d tex3d left a comment

Choose a reason for hiding this comment

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

I think this looks good, with just one concern:

The signed form of the i32 opcode is the one llvm will print and the one that will be needed in CHECK lines of tests. I think it would be best to stick to the signed form for input IR as well, so that each opcode doesn't have two textural forms to search/replace when updating ops in tests for released versions.

Copy link
Contributor

@tex3d tex3d left a comment

Choose a reason for hiding this comment

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

I added suggested change comments to make updating the opcodes to canonical form easier.


; Function Attrs: nounwind
define void @"\01?test_closesthit@@YAXUPayload@@UBuiltInTriangleIntersectionAttributes@@@Z"(%struct.Payload* noalias nocapture %payload, %struct.BuiltInTriangleIntersectionAttributes* nocapture readnone %attr) #0 {
%1 = call i32 @dx.op.clusterID(i32 2147483651) ; ClusterID()
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
%1 = call i32 @dx.op.clusterID(i32 2147483651) ; ClusterID()
%1 = call i32 @dx.op.clusterID(i32 -2147483645) ; ClusterID()

Comment on lines +22 to +26
%1 = call %dx.types.HitObject @dx.op.hitObject_MakeNop(i32 266) ; HitObject_MakeNop()
%2 = call i32 @dx.op.hitObject_StateScalar.i32(i32 2147483654, %dx.types.HitObject %1) ; HitObject_ClusterID(hitObject)
%3 = call i32 @dx.op.allocateRayQuery(i32 178, i32 0) ; AllocateRayQuery(constRayFlags)
%4 = call i32 @dx.op.rayQuery_StateScalar.i32(i32 2147483652, i32 %3) ; RayQuery_CandidateClusterID(rayQueryHandle)
%5 = call i32 @dx.op.rayQuery_StateScalar.i32(i32 2147483653, i32 %3) ; RayQuery_CommittedClusterID(rayQueryHandle)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
%1 = call %dx.types.HitObject @dx.op.hitObject_MakeNop(i32 266) ; HitObject_MakeNop()
%2 = call i32 @dx.op.hitObject_StateScalar.i32(i32 2147483654, %dx.types.HitObject %1) ; HitObject_ClusterID(hitObject)
%3 = call i32 @dx.op.allocateRayQuery(i32 178, i32 0) ; AllocateRayQuery(constRayFlags)
%4 = call i32 @dx.op.rayQuery_StateScalar.i32(i32 2147483652, i32 %3) ; RayQuery_CandidateClusterID(rayQueryHandle)
%5 = call i32 @dx.op.rayQuery_StateScalar.i32(i32 2147483653, i32 %3) ; RayQuery_CommittedClusterID(rayQueryHandle)
%1 = call %dx.types.HitObject @dx.op.hitObject_MakeNop(i32 266) ; HitObject_MakeNop()
%2 = call i32 @dx.op.hitObject_StateScalar.i32(i32 -2147483642, %dx.types.HitObject %1) ; HitObject_ClusterID(hitObject)
%3 = call i32 @dx.op.allocateRayQuery(i32 178, i32 0) ; AllocateRayQuery(constRayFlags)
%4 = call i32 @dx.op.rayQuery_StateScalar.i32(i32 -2147483644, i32 %3) ; RayQuery_CandidateClusterID(rayQueryHandle)
%5 = call i32 @dx.op.rayQuery_StateScalar.i32(i32 -2147483643, i32 %3) ; RayQuery_CommittedClusterID(rayQueryHandle)


; Function Attrs: nounwind
define void @"\01?test_closesthit@@YAXUPayload@@UBuiltInTriangleIntersectionAttributes@@@Z"(%struct.Payload* noalias nocapture %payload, %struct.BuiltInTriangleIntersectionAttributes* nocapture readnone %attr) #0 {
%1 = call <9 x float> @dx.op.triangleObjectPosition.f32(i32 2147483655) ; TriangleObjectPosition()
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
%1 = call <9 x float> @dx.op.triangleObjectPosition.f32(i32 2147483655) ; TriangleObjectPosition()
%1 = call <9 x float> @dx.op.triangleObjectPosition.f32(i32 -2147483641) ; TriangleObjectPosition()

Comment on lines +22 to +26
%1 = call %dx.types.HitObject @dx.op.hitObject_MakeNop(i32 266) ; HitObject_MakeNop()
%2 = call <9 x float> @dx.op.hitObject_TriangleObjectPosition.f32(i32 2147483658, %dx.types.HitObject %1) ; HitObject_TriangleObjectPosition(hitObject)
%3 = call i32 @dx.op.allocateRayQuery(i32 178, i32 0) ; AllocateRayQuery(constRayFlags)
%4 = call <9 x float> @dx.op.rayQuery_CandidateTriangleObjectPosition.f32(i32 2147483656, i32 %3) ; RayQuery_CandidateTriangleObjectPosition(rayQueryHandle)
%5 = call <9 x float> @dx.op.rayQuery_CommittedTriangleObjectPosition.f32(i32 2147483657, i32 %3) ; RayQuery_CommittedTriangleObjectPosition(rayQueryHandle)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
%1 = call %dx.types.HitObject @dx.op.hitObject_MakeNop(i32 266) ; HitObject_MakeNop()
%2 = call <9 x float> @dx.op.hitObject_TriangleObjectPosition.f32(i32 2147483658, %dx.types.HitObject %1) ; HitObject_TriangleObjectPosition(hitObject)
%3 = call i32 @dx.op.allocateRayQuery(i32 178, i32 0) ; AllocateRayQuery(constRayFlags)
%4 = call <9 x float> @dx.op.rayQuery_CandidateTriangleObjectPosition.f32(i32 2147483656, i32 %3) ; RayQuery_CandidateTriangleObjectPosition(rayQueryHandle)
%5 = call <9 x float> @dx.op.rayQuery_CommittedTriangleObjectPosition.f32(i32 2147483657, i32 %3) ; RayQuery_CommittedTriangleObjectPosition(rayQueryHandle)
%1 = call %dx.types.HitObject @dx.op.hitObject_MakeNop(i32 266) ; HitObject_MakeNop()
%2 = call <9 x float> @dx.op.hitObject_TriangleObjectPosition.f32(i32 -2147483638, %dx.types.HitObject %1) ; HitObject_TriangleObjectPosition(hitObject)
%3 = call i32 @dx.op.allocateRayQuery(i32 178, i32 0) ; AllocateRayQuery(constRayFlags)
%4 = call <9 x float> @dx.op.rayQuery_CandidateTriangleObjectPosition.f32(i32 -2147483640, i32 %3) ; RayQuery_CandidateTriangleObjectPosition(rayQueryHandle)
%5 = call <9 x float> @dx.op.rayQuery_CommittedTriangleObjectPosition.f32(i32 -2147483639, i32 %3) ; RayQuery_CommittedTriangleObjectPosition(rayQueryHandle)

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.

3 participants