-
Notifications
You must be signed in to change notification settings - Fork 825
[SM6.10] VEC9 TriangleObjectPositions / CHECK-pass validation tests #7831
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
base: main
Are you sure you want to change the base?
Conversation
|
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 |
|
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 |
|
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 |
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. |
tex3d
left a comment
There was a problem hiding this 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.
|
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 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. |
|
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. |
SM6.10 tracking bug: microsoft#7824
b3a793f to
63a5b8b
Compare
tex3d
left a comment
There was a problem hiding this 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.
tex3d
left a comment
There was a problem hiding this 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.
Co-authored-by: Tex Riddell <texr@microsoft.com>
Co-authored-by: Tex Riddell <texr@microsoft.com>
Co-authored-by: Tex Riddell <texr@microsoft.com>
Co-authored-by: Tex Riddell <texr@microsoft.com>
SM6.10 tracking bug: #7824