-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[HLSL] Implement D3DCOLORtoUBYTE4 intrinsic #122202
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 3 commits
5610b22
e2c4abd
57f9048
6d17064
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 |
---|---|---|
|
@@ -41,6 +41,14 @@ constexpr enable_if_t<sizeof(U) == sizeof(T), U> bit_cast(T F) { | |
return __builtin_bit_cast(U, F); | ||
} | ||
|
||
constexpr vector<uint, 4> d3d_color_to_ubyte4_impl(vector<float, 4> V) { | ||
// Use the same scaling factor used by FXC (i.e., 255.001953) | ||
// Excerpt from stackoverflow discussion: | ||
// "Built-in rounding, necessary because of truncation. 0.001953 * 256 = 0.5" | ||
// https://stackoverflow.com/questions/52103720/why-does-d3dcolortoubyte4-multiplies-components-by-255-001953f | ||
return V.zyxw * 255.001953f; | ||
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 implementation of 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. I think the change from DXC for SPIR-V is fine. The DXIL behaviour is the correct behaviour. |
||
} | ||
|
||
template <typename T> | ||
constexpr enable_if_t<is_same<float, T>::value || is_same<half, T>::value, T> | ||
length_impl(T X) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
// RUN: %clang_cc1 -finclude-default-header -triple \ | ||
// RUN: dxil-pc-shadermodel6.3-library %s \ | ||
// RUN: -emit-llvm -O1 -o - | FileCheck %s --check-prefixes=CHECK | ||
|
||
// CHECK-LABEL: D3DCOLORtoUBYTE4 | ||
int4 test_D3DCOLORtoUBYTE4(float4 p1) { | ||
// CHECK: %[[SCALED:.*]] = fmul [[FMFLAGS:.*]][[FLOAT_TYPE:<4 x float>]] %{{.*}}, splat (float 0x406FE01000000000) | ||
// CHECK: %[[CONVERTED:.*]] = fptoui [[FLOAT_TYPE]] %[[SCALED]] to [[INT_TYPE:<4 x i32>]] | ||
// CHECK: %[[SHUFFLED:.*]] = shufflevector [[INT_TYPE]] %[[CONVERTED]], [[INT_TYPE]] poison, <4 x i32> <i32 2, i32 1, i32 0, i32 3> | ||
// CHECK: ret [[INT_TYPE]] %[[SHUFFLED]] | ||
return D3DCOLORtoUBYTE4(p1); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
// RUN: %clang_cc1 -finclude-default-header -triple dxil-pc-shadermodel6.6-library %s -emit-llvm-only -disable-llvm-passes -verify | ||
|
||
int4 test_too_few_arg() { | ||
return D3DCOLORtoUBYTE4(); | ||
// expected-error@-1 {{no matching function for call to 'D3DCOLORtoUBYTE4'}} | ||
// expected-note@hlsl/hlsl_intrinsics.h:* {{candidate function not viable: requires single argument 'V', but no arguments were provided}} | ||
} | ||
|
||
int4 test_too_many_arg(float4 v) { | ||
return D3DCOLORtoUBYTE4(v, v); | ||
// expected-error@-1 {{no matching function for call to 'D3DCOLORtoUBYTE4'}} | ||
// expected-note@hlsl/hlsl_intrinsics.h:* {{candidate function not viable: requires single argument 'V', but 2 arguments were provided}} | ||
} | ||
|
||
int4 float2_arg(float2 v) { | ||
return D3DCOLORtoUBYTE4(v); | ||
// expected-error@-1 {{no matching function for call to 'D3DCOLORtoUBYTE4'}} | ||
// expected-note@hlsl/hlsl_intrinsics.h:* {{candidate function not viable: no known conversion from 'vector<[...], 2>' to 'vector<[...], 4>' for 1st argument}} | ||
} | ||
|
||
struct S { | ||
float4 f; | ||
}; | ||
|
||
int4 struct_arg(S v) { | ||
return D3DCOLORtoUBYTE4(v); | ||
// expected-error@-1 {{no matching function for call to 'D3DCOLORtoUBYTE4'}} | ||
// expected-note@hlsl/hlsl_intrinsics.h:* {{candidate function not viable: no known conversion from 'S' to 'vector<float, 4>' (vector of 4 'float' values) for 1st argument}} | ||
} |
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.
It might be better to refer to the implementation in DXC here, rather than copying the comment in DXC about compatibility with FXC.
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 revised the comment to mention that the scaling factor is the same as FXC and also the DXC DXIL implementation.
I also kept the link to the stackoverflow discussion, as the comments in the DXC DXIL implementation referred to it but did not provide a link, which I think is kind of important.
Are these new comments more suitable? Or should I just simply just provide a link to the DXC implementation and cut out the repetition?