-
Notifications
You must be signed in to change notification settings - Fork 851
[SM6.10] Implement LinAlg Convert Builtin #8308
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| // REQUIRES: dxil-1-10 | ||
| // RUN: %dxc -T cs_6_10 -HV 202x -E main %s | FileCheck %s | ||
| // RUN: %dxc -T cs_6_10 -HV 202x -E main -fcgl %s | FileCheck %s --check-prefix=CHECK2 | ||
|
|
||
| [numthreads(4,1,1)] | ||
| void main() { | ||
| // CHECK-LABEL: define void @main() | ||
|
|
||
| // CHECK: %{{.*}} = call <4 x i32> @dx.op.linAlgConvert.v4i32.v4f32 | ||
| // CHECK-SAME: (i32 -2147483618, <4 x float> <float 9.000000e+00, float 8.000000e+00, float 7.000000e+00, float 6.000000e+00>, i32 1, i32 2) | ||
| // CHECK-SAME: ; LinAlgConvert(inputVector,inputInterpretation,outputInterpretation) | ||
|
|
||
| // CHECK2: call void @"dx.hl.op..void (i32, <4 x i32>*, <4 x float>, i32, i32)" | ||
| // CHECK2-SAME: (i32 422, <4 x i32>* %result, <4 x float> %{{.*}}, i32 1, i32 2) | ||
| float4 vec = {9.0, 8.0, 7.0, 6.0}; | ||
| int4 result; | ||
| __builtin_LinAlg_Convert(result, vec, 1, 2); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -50,6 +50,9 @@ define void @mainAS() { | |
| ; dx.op.linAlgMatVecMulAdd | ||
| %v7 = call <4 x i32> @dx.op.linAlgMatVecMulAdd.v4i32.mC4M5N4U0S2.v4i32.v4i32(i32 -2147483622, %dx.types.LinAlgMatrixC4M5N4U0S2 %v4, <4 x i32> <i32 9, i32 9, i32 9, i32 9>, i32 2, <4 x i32> <i32 7, i32 7, i32 7, i32 7>, i32 3) ; LinAlgMatVecMulAdd(matrix,inputVector,inputInterpretation,biasVector,biasInterpretation) | ||
|
|
||
| ; dx.op.linAlgConvert | ||
| %v16 = call <4 x float> @dx.op.linAlgConvert.v4f32.v4i32(i32 -2147483618, <4 x i32> zeroinitializer, i32 1, i32 2) ; LinAlgConvert(inputVector,inputInterpretation,outputInterpretation) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One thing that bugs me about these tests is that the interpretation values are not valid for the overload types. When we add actual validation, we'd have to update all these tests to use valid interpretation values. For example,
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah. We've been lax with the precise details. Mainly because these tests are testing other parts of the op. As you mentioned, we will be forced to fix these eventually so they aren't going to be forgotten but if you want me fix them now I can
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is part of the general feedback that doesn’t need to be addressed now. |
||
|
|
||
| ; | ||
| ; Built-ins restricted to compute, mesh and amplification shaders | ||
| ; | ||
|
|
@@ -123,6 +126,9 @@ declare <4 x i32> @dx.op.linAlgMatVecMul.v4i32.mC4M5N4U0S2.v4i32(i32, %dx.types. | |
| ; Function Attrs: nounwind | ||
| declare <4 x i32> @dx.op.linAlgMatVecMulAdd.v4i32.mC4M5N4U0S2.v4i32.v4i32(i32, %dx.types.LinAlgMatrixC4M5N4U0S2, <4 x i32>, i32, <4 x i32>, i32) #0 | ||
|
|
||
| ; Function Attrs: nounwind | ||
| declare <4 x float> @dx.op.linAlgConvert.v4f32.v4i32(i32, <4 x i32>, i32, i32) #0 | ||
|
|
||
| ; Function Attrs: nounwind | ||
| declare %dx.types.LinAlgMatrixC4M4N5U1S2 @dx.op.linAlgCopyConvertMatrix.mC4M4N5U1S2.mC4M5N4U0S2(i32, %dx.types.LinAlgMatrixC4M5N4U0S2, i1) #0 | ||
|
|
||
|
|
||
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.
Just a general comment:
Normally, validation tests are meant to check that the validator catches specific invalid cases, and valid cases are typically tested elsewhere where we expect to produce specific IR outputs and pass validation.
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.
This is a bit of a tricky one. These tests are testing invalid cases (the ops that aren't allowed in these shaders) but for completeness the ops all ops were listed here, including the allowed ones. If this feels inappropriate, we could remove all the allowed-everywhere entries from these
.lltests in a separate PR. But I don't think it makes sense to do here. In this PR, the new op is added to keep the files consistent.lmk what you'd prefer. I can file an issue to remove the allowed ops if that's the route you want to take
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.
This is part of the general feedback that doesn’t need to be addressed now.
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.
#8315