Adding asuint implementation to hlsl
#107292
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
joaosaffran
force-pushed
the
intrinsics/asuint
branch
5 times, most recently
from
b06b483 to
3118d36
Compare
farzonl
reviewed
Sep 5, 2024
joaosaffran
force-pushed
the
intrinsics/asuint
branch
3 times, most recently
from
79e2f32 to
7b15462
Compare
llvmbot
added
clang
|
@llvm/pr-subscribers-hlsl @llvm/pr-subscribers-backend-x86 Author: None (joaosaffran) ChangesImplements support for the Fixes: #70097 Full diff: https://github.com/llvm/llvm-project/pull/107292.diff 6 Files Affected:
diff --git a/clang/include/clang/Basic/Builtins.td b/clang/include/clang/Basic/Builtins.td
index 9e2a590f265ac8..38de1df11b7b5a 100644
--- a/clang/include/clang/Basic/Builtins.td
+++ b/clang/include/clang/Basic/Builtins.td
@@ -4745,6 +4745,12 @@ def HLSLRcp : LangBuiltin<"HLSL_LANG"> {
let Prototype = "void(...)";
}
+def HLSLAsUint : LangBuiltin<"HLSL_LANG"> {
+ let Spellings = ["__builtin_hlsl_elementwise_asuint"];
+ let Attributes = [NoThrow, Const];
+ let Prototype = "void(...)";
+}
+
def HLSLRSqrt : LangBuiltin<"HLSL_LANG"> {
let Spellings = ["__builtin_hlsl_elementwise_rsqrt"];
let Attributes = [NoThrow, Const];
diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index e826c1c6fbbd23..035858e7e291b0 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -18812,6 +18812,19 @@ case Builtin::BI__builtin_hlsl_elementwise_isinf: {
llvm::FunctionType::get(IntTy, {}, false), "__hlsl_wave_get_lane_index",
{}, false, true));
}
+ case Builtin::BI__builtin_hlsl_elementwise_asuint: {
+ Value *Op = EmitScalarExpr(E->getArg(0)->IgnoreImpCasts());
+
+ llvm::Type *DestTy = llvm::Type::getInt32Ty(this->getLLVMContext());
+
+ if (Op->getType()->isVectorTy()) {
+ const VectorType *VecTy = E->getArg(0)->getType()->getAs<VectorType>();
+ DestTy = llvm::VectorType::get(
+ DestTy, ElementCount::getFixed(VecTy->getNumElements()));
+ }
+
+ return Builder.CreateBitCast(Op, DestTy);
+ }
case Builtin::BI__builtin_hlsl_wave_is_first_lane: {
Intrinsic::ID ID = CGM.getHLSLRuntime().getWaveIsFirstLaneIntrinsic();
return EmitRuntimeCall(Intrinsic::getDeclaration(&CGM.getModule(), ID));
diff --git a/clang/lib/Headers/hlsl/hlsl_intrinsics.h b/clang/lib/Headers/hlsl/hlsl_intrinsics.h
index 5c08a45a35377d..f40469937ddc38 100644
--- a/clang/lib/Headers/hlsl/hlsl_intrinsics.h
+++ b/clang/lib/Headers/hlsl/hlsl_intrinsics.h
@@ -387,6 +387,23 @@ float3 asin(float3);
_HLSL_BUILTIN_ALIAS(__builtin_elementwise_asin)
float4 asin(float4);
+//===----------------------------------------------------------------------===//
+// asuint builtins
+//===----------------------------------------------------------------------===//
+
+/// \fn uint asuint(T Val)
+/// \brief Interprets the bit pattern of x as an unsigned integer.
+/// \param Val The input value.
+
+_HLSL_BUILTIN_ALIAS(__builtin_hlsl_elementwise_asuint)
+uint asuint(float);
+_HLSL_BUILTIN_ALIAS(__builtin_hlsl_elementwise_asuint)
+uint2 asuint(float2);
+_HLSL_BUILTIN_ALIAS(__builtin_hlsl_elementwise_asuint)
+uint3 asuint(float3);
+_HLSL_BUILTIN_ALIAS(__builtin_hlsl_elementwise_asuint)
+uint4 asuint(float4);
+
//===----------------------------------------------------------------------===//
// atan builtins
//===----------------------------------------------------------------------===//
diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp
index 778d524a005482..3adf571c75f7b0 100644
--- a/clang/lib/Sema/SemaHLSL.cpp
+++ b/clang/lib/Sema/SemaHLSL.cpp
@@ -12,6 +12,7 @@
#include "clang/AST/Decl.h"
#include "clang/AST/Expr.h"
#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/Type.h"
#include "clang/Basic/DiagnosticSema.h"
#include "clang/Basic/LLVM.h"
#include "clang/Basic/SourceLocation.h"
@@ -1401,6 +1402,25 @@ bool CheckVectorElementCallArgs(Sema *S, CallExpr *TheCall) {
return true;
}
+bool CheckArgTypeWithoutImplicits(
+ Sema *S, Expr *Arg, QualType ExpectedType,
+ llvm::function_ref<bool(clang::QualType PassedType)> Check) {
+
+ QualType ArgTy = Arg->IgnoreImpCasts()->getType();
+
+ clang::QualType BaseType =
+ ArgTy->isVectorType()
+ ? ArgTy->getAs<clang::VectorType>()->getElementType()
+ : ArgTy;
+
+ if (Check(BaseType)) {
+ S->Diag(Arg->getBeginLoc(), diag::err_typecheck_convert_incompatible)
+ << ArgTy << ExpectedType << 1 << 0 << 0;
+ return true;
+ }
+ return false;
+}
+
bool CheckArgsTypesAreCorrect(
Sema *S, CallExpr *TheCall, QualType ExpectedType,
llvm::function_ref<bool(clang::QualType PassedType)> Check) {
@@ -1427,6 +1447,22 @@ bool CheckAllArgsHaveFloatRepresentation(Sema *S, CallExpr *TheCall) {
checkAllFloatTypes);
}
+bool CheckArgIsFloatOrIntWithoutImplicits(Sema *S, Expr *Arg) {
+ auto checkFloat = [](clang::QualType PassedType) -> bool {
+ return !PassedType->isFloat32Type() && !PassedType->isIntegerType();
+ };
+
+ return CheckArgTypeWithoutImplicits(S, Arg, S->Context.FloatTy, checkFloat);
+}
+
+bool CheckArgIsIntegerWithoutImplicits(Sema *S, Expr *Arg) {
+ auto checkFloat = [](clang::QualType PassedType) -> bool {
+ return !PassedType->isIntegerType();
+ };
+
+ return CheckArgTypeWithoutImplicits(S, Arg, S->Context.FloatTy, checkFloat);
+}
+
bool CheckFloatOrHalfRepresentations(Sema *S, CallExpr *TheCall) {
auto checkFloatorHalf = [](clang::QualType PassedType) -> bool {
clang::QualType BaseType =
@@ -1581,6 +1617,16 @@ bool SemaHLSL::CheckBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall) {
return true;
break;
}
+ case Builtin::BI__builtin_hlsl_elementwise_asuint: {
+ if (SemaRef.checkArgCount(TheCall, 1))
+ return true;
+
+ Expr *Arg = TheCall->getArg(0);
+ if (CheckArgIsFloatOrIntWithoutImplicits(&SemaRef, Arg))
+ return true;
+
+ break;
+ }
case Builtin::BI__builtin_elementwise_acos:
case Builtin::BI__builtin_elementwise_asin:
case Builtin::BI__builtin_elementwise_atan:
diff --git a/clang/test/CodeGenHLSL/builtins/asuint.hlsl b/clang/test/CodeGenHLSL/builtins/asuint.hlsl
new file mode 100644
index 00000000000000..2ae7d8219ac671
--- /dev/null
+++ b/clang/test/CodeGenHLSL/builtins/asuint.hlsl
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -finclude-default-header -x hlsl -triple dxil-pc-shadermodel6.3-library %s -fnative-half-type -emit-llvm -disable-llvm-passes -o - | FileCheck %s
+
+
+// CHECK-LABEL: test_asuint4_uint
+// CHECK: ret i32 %0
+export uint test_asuint4_uint(uint p0) {
+ return asuint(p0);
+}
+
+// CHECK-LABEL: test_asuint4_int
+// CHECK: %splat.splatinsert = insertelement <4 x i32> poison, i32 %0, i64 0
+export uint4 test_asuint4_int(int p0) {
+ return asuint(p0);
+}
+
+// CHECK-LABEL: test_asuint_float
+// CHECK: %1 = bitcast float %0 to i32
+export uint test_asuint_float(float p0) {
+ return asuint(p0);
+}
+
+// CHECK-LABEL: test_asuint_float
+// CHECK: %1 = bitcast <4 x float> %0 to <4 x i32>
+export uint4 test_asuint_float4(float4 p0) {
+ return asuint(p0);
+}
\ No newline at end of file
diff --git a/clang/test/SemaHLSL/BuiltIns/asuint-errors.hlsl b/clang/test/SemaHLSL/BuiltIns/asuint-errors.hlsl
new file mode 100644
index 00000000000000..e9da975bff1b5e
--- /dev/null
+++ b/clang/test/SemaHLSL/BuiltIns/asuint-errors.hlsl
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -finclude-default-header -triple dxil-pc-shadermodel6.6-library %s -fnative-half-type -emit-llvm-only -disable-llvm-passes -verify -verify-ignore-unexpected
+
+
+export uint4 test_asuint_too_many_arg(float p0, float p1) {
+ return __builtin_hlsl_elementwise_asuint(p0, p1);
+ // expected-error@-1 {{too many arguments to function call, expected 1, have 2}}
+}
+
+
+export uint fn(double p1) {
+ return asuint(p1);
+ // expected-error@-1 {{passing 'double' to parameter of incompatible type 'float'}}
+}
+
+export uint fn(half p1) {
+ return asuint(p1);
+ // expected-error@-1 {{passing 'half' to parameter of incompatible type 'float'}}
+}
|
joaosaffran
force-pushed
the
intrinsics/asuint
branch
from
7b15462 to
500590e
Compare
farzonl
reviewed
Sep 6, 2024
farzonl
reviewed
Sep 6, 2024
farzonl
reviewed
Sep 6, 2024
bogner
reviewed
Sep 6, 2024
There was a problem hiding this comment.
This is going in the right direction but the tests need some work. I made a couple of comments to point you in the right direction.
We also need to make sure we're testing all of the cases we care about, so we need tests with inputs of int, uint, vector of int, vector of uint, float, vector of float. You've covered some but not all of these.
joaosaffran
force-pushed
the
intrinsics/asuint
branch
from
500590e to
26577ea
Compare
llvm-beanz
reviewed
Sep 6, 2024
There was a problem hiding this comment.
One question I have about this is do we actually need new builtins? These are just doing bit casts correct?
Could we implement them entirely in the header? Something like:
template <typename T, typename U>
T bit_cast(U Val) { return __builtin_bit_cast(T, Val); }
uint asuint(float F) { return bit_cast<uint>(F); }This seems to work for me in godbolt: https://godbolt.org/z/8b61a1WMr
We may want to apply the [[alwaysinline]] attribute, but it seems like a reasonable approach. Thoughts?
joaosaffran
force-pushed
the
intrinsics/asuint
branch
3 times, most recently
from
e1b6e3b to
d0619c7
Compare
bogner
reviewed
Sep 9, 2024
| @@ -0,0 +1,18 @@ | |||
| // RUN: %clang_cc1 -finclude-default-header -triple dxil-pc-shadermodel6.6-library %s -fnative-half-type -emit-llvm-only -disable-llvm-passes -verify -verify-ignore-unexpected | |||
There was a problem hiding this comment.
I don't think we should be using -verify-ignore-unexpected here - it makes it harder to understand what's going wrong if the test fails, and generally we shouldn't have errors that we're not checking for in a test.
There was a problem hiding this comment.
This still has -verify-ignore-unexpected...
There was a problem hiding this comment.
I am seeing some issues when testing without it, for example:
error: 'expected-error' diagnostics seen but not expected
The error messages are really verbose. What is the best approach here: i) add the verbose lines?; ii) ignore the unexpected and check only one of the errors?
There was a problem hiding this comment.
what messages are seen but not expected?
There was a problem hiding this comment.
I've updated the tests, but here is the full error message, for such tests:
/workspace/llvm-project/build-debug/bin/clang -cc1 -internal-isystem /workspace/llvm-project/build-debug/lib/clang/20/include -nostdsysteminc -finclude-default-header -x hlsl -triple dxil-pc-shadermodel6.3-library /workspace/llvm-project/clang/test/SemaHLSL/BuiltIns/asuint-errors.hlsl -fnative-half-type -emit-llvm
[12/12] Creating executable symlink bin/clang
/workspace/llvm-project/clang/test/SemaHLSL/BuiltIns/asuint-errors.hlsl:5:10: error: no matching function for call to 'asuint'
5 | return asuint(p0, p1);
| ^~~~~~
/workspace/llvm-project/build-debug/lib/clang/20/include/hlsl/hlsl_intrinsics.h:401:30: note: candidate function template not viable: requires single argument 'V', but 2 arguments were provided
401 | _HLSL_INLINE vector<uint, N> asuint(vector<T, N> V) {
| ^ ~~~~~~~~~~~~~~
/workspace/llvm-project/build-debug/lib/clang/20/include/hlsl/hlsl_intrinsics.h:405:41: note: candidate function template not viable: requires single argument 'F', but 2 arguments were provided
405 | template <typename T> _HLSL_INLINE uint asuint(T F) {
| ^ ~~~
/workspace/llvm-project/build-debug/lib/clang/20/include/hlsl/hlsl_intrinsics.h:406:10: error: no matching function for call to 'bit_cast'
406 | return __detail::bit_cast<uint, T>(F);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
/workspace/llvm-project/clang/test/SemaHLSL/BuiltIns/asuint-errors.hlsl:12:12: note: in instantiation of function template specialization 'hlsl::asuint<double>' requested here
12 | return asuint(p1);
| ^
/workspace/llvm-project/build-debug/lib/clang/20/include/hlsl/hlsl_detail.h:27:1: note: candidate template ignored: could not match 'vector<double, N>' against 'double'
27 | bit_cast(vector<T, N> V) {
| ^
/workspace/llvm-project/build-debug/lib/clang/20/include/hlsl/hlsl_detail.h:32:66: note: candidate template ignored: substitution failure [with U = uint, T = double]: no type named 'Type' in 'hlsl::__detail::enable_if<false, unsigned int>'
32 | _HLSL_INLINE typename enable_if<sizeof(U) == sizeof(T), U>::Type bit_cast(T F) {
| ~~~~~~~~ ^
In file included from <built-in>:1:
In file included from /workspace/llvm-project/build-debug/lib/clang/20/include/hlsl.h:13:
/workspace/llvm-project/build-debug/lib/clang/20/include/hlsl/hlsl_intrinsics.h:406:10: error: no matching function for call to 'bit_cast'
406 | return __detail::bit_cast<uint, T>(F);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
/workspace/llvm-project/clang/test/SemaHLSL/BuiltIns/asuint-errors.hlsl:20:12: note: in instantiation of function template specialization 'hlsl::asuint<half>' requested here
20 | return asuint(p1);
| ^
/workspace/llvm-project/build-debug/lib/clang/20/include/hlsl/hlsl_detail.h:27:1: note: candidate template ignored: could not match 'vector<half, N>' against 'half'
27 | bit_cast(vector<T, N> V) {
| ^
/workspace/llvm-project/build-debug/lib/clang/20/include/hlsl/hlsl_detail.h:32:66: note: candidate template ignored: substitution failure [with U = uint, T = half]: no type named 'Type' in 'hlsl::__detail::enable_if<false, unsigned int>'
32 | _HLSL_INLINE typename enable_if<sizeof(U) == sizeof(T), U>::Type bit_cast(T F) {
| ~~~~~~~~ ^
3 errors generated.
joaosaffran
force-pushed
the
intrinsics/asuint
branch
from
472c796 to
218d45f
Compare
|
After discussing with @bogner and @farzonl, we decided the following:
|
joaosaffran
force-pushed
the
intrinsics/asuint
branch
4 times, most recently
from
b9fbd1f to
d6cc453
Compare
farzonl
reviewed
Sep 12, 2024
farzonl
reviewed
Sep 12, 2024
llvm-beanz
reviewed
Sep 12, 2024
joaosaffran
force-pushed
the
intrinsics/asuint
branch
2 times, most recently
from
d9fa028 to
d2f55b3
Compare
bogner
reviewed
Sep 12, 2024
| @@ -0,0 +1,18 @@ | |||
| // RUN: %clang_cc1 -finclude-default-header -triple dxil-pc-shadermodel6.6-library %s -fnative-half-type -emit-llvm-only -disable-llvm-passes -verify -verify-ignore-unexpected | |||
There was a problem hiding this comment.
This still has -verify-ignore-unexpected...
You can test this locally with the following command:git-clang-format --diff 447b32fb192b90be00164a027f990e16c0325de3 4559fac3ef10d70c8cbf7b9d5eef1884dcb04ef0 --extensions h -- clang/lib/Headers/hlsl/hlsl_detail.h clang/lib/Headers/hlsl/hlsl_intrinsics.hView the diff from clang-format here.diff --git a/clang/lib/Headers/hlsl/hlsl_detail.h b/clang/lib/Headers/hlsl/hlsl_detail.h
index 9801d86208..04ee784a76 100644
--- a/clang/lib/Headers/hlsl/hlsl_detail.h
+++ b/clang/lib/Headers/hlsl/hlsl_detail.h
@@ -23,7 +23,7 @@ template <typename T> struct enable_if<true, T> {
};
template <typename U, typename T, int N>
-_HLSL_INLINE typename enable_if<sizeof(U) == sizeof(T), vector<U, N> >::Type
+_HLSL_INLINE typename enable_if<sizeof(U) == sizeof(T), vector<U, N>>::Type
bit_cast(vector<T, N> V) {
return __builtin_bit_cast(vector<U, N>, V);
}
|
joaosaffran
force-pushed
the
intrinsics/asuint
branch
from
d2f55b3 to
5df40b1
Compare
joaosaffran
force-pushed
the
intrinsics/asuint
branch
from
5df40b1 to
4559fac
Compare
3 tasks
This formatting issue is caused because, clang's LLVM style assumes C++14, where the space isn't required, but HLSL is C++98 where the space is required. This issue will be addressed once we address all fixes for future HLSL releases. |
farzonl
approved these changes
Sep 13, 2024
Co-authored-by: Justin Bogner <[email protected]>
|
@joaosaffran Congratulations on having your first Pull Request (PR) merged into the LLVM Project! Your changes will be combined with recent changes from other authors, then tested by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR. Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues. How to do this, and the rest of the post-merge process, is covered in detail here. If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again. If you don't get any reports, no action is required from you. Your changes are working as expected, well done! |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/27/builds/438 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Implements support for the
asuintHLSL function casting behaviour.Addressing the
splitdoublescenario will be addressed in a future PR.Fixes: #70097