[HLSL] [DXIL] Implement the AddUint64 HLSL function and the UAddc DXIL op
#125319
There are no files selected for viewing
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19105,6 +19105,51 @@ Value *CodeGenFunction::EmitHLSLBuiltinExpr(unsigned BuiltinID, | |
| return nullptr; | ||
|
|
||
| switch (BuiltinID) { | ||
| case Builtin::BI__builtin_hlsl_adduint64: { | ||
|
There was a problem hiding this comment. I suppose I know why we weren't able to re-use the There was a problem hiding this comment. Ah, I see that you have it in the commit notes. I still think it would be worth noting with more context here There was a problem hiding this comment.
Since pointers are not supported in HLSL, an error is emitted when running HLSL codegen tests with an example implementation like the following in _HLSL_AVAILABILITY(shadermodel, 6.0)
const inline uint32_t2 AddUint64(uint32_t2 a, uint32_t2 b) {
uint32_t carry;
uint32_t low_sum = __builtin_addc(a.x, b.x, 0, &carry);
uint32_t high_sum = __builtin_addc(a.y, b.y, carry, nullptr);
return uint32_t2(low_sum, high_sum);
}There was a problem hiding this comment. So while HLSL does not support pointers we do have a concept of out args. if you search for There was a problem hiding this comment. Do you think it is something I should do for this implementation? There was a problem hiding this comment. So the downside of what I suggested is that it would be a hybrid solution. You are writing the algorithm in HLSL, but you are also massaging the codegen to do out args instead of pointers, and write sema checks because we have to introduce a new builtin. My thinking was there would be less total codgen if we did it the way I suggested and some of the sema checks would benefit from language rules instead of us having to put a bunch of effort into |
||
| Value *OpA = EmitScalarExpr(E->getArg(0)); | ||
| Value *OpB = EmitScalarExpr(E->getArg(1)); | ||
| assert(E->getArg(0)->getType()->hasIntegerRepresentation() && | ||
| E->getArg(1)->getType()->hasIntegerRepresentation() && | ||
| "AddUint64 operands must have an integer representation"); | ||
| assert(((E->getArg(0)->getType()->castAs<VectorType>()->getNumElements() == | ||
| 2 && | ||
| E->getArg(1)->getType()->castAs<VectorType>()->getNumElements() == | ||
| 2) || | ||
| (E->getArg(0)->getType()->castAs<VectorType>()->getNumElements() == | ||
| 4 && | ||
| E->getArg(1)->getType()->castAs<VectorType>()->getNumElements() == | ||
| 4)) && | ||
| "input vectors must have 2 or 4 elements each"); | ||
|
|
||
| llvm::Value *Result = PoisonValue::get(OpA->getType()); | ||
| uint64_t NumElements = | ||
| E->getArg(0)->getType()->castAs<VectorType>()->getNumElements(); | ||
| for (uint64_t i = 0; i < NumElements / 2; ++i) { | ||
|
|
||
| // Obtain low and high words of inputs A and B | ||
| llvm::Value *LowA = Builder.CreateExtractElement(OpA, 2 * i + 0); | ||
| llvm::Value *HighA = Builder.CreateExtractElement(OpA, 2 * i + 1); | ||
| llvm::Value *LowB = Builder.CreateExtractElement(OpB, 2 * i + 0); | ||
| llvm::Value *HighB = Builder.CreateExtractElement(OpB, 2 * i + 1); | ||
|
|
||
| // Use an uadd_with_overflow to compute the sum of low words and obtain a | ||
| // carry value | ||
| llvm::Value *Carry; | ||
| llvm::Value *LowSum = EmitOverflowIntrinsic( | ||
| *this, llvm::Intrinsic::uadd_with_overflow, LowA, LowB, Carry); | ||
| llvm::Value *ZExtCarry = Builder.CreateZExt(Carry, HighA->getType()); | ||
|
|
||
| // Sum the high words and the carry | ||
| llvm::Value *HighSum = Builder.CreateAdd(HighA, HighB); | ||
| llvm::Value *HighSumPlusCarry = Builder.CreateAdd(HighSum, ZExtCarry); | ||
|
|
||
| // Insert the low and high word sums into the result vector | ||
| Result = Builder.CreateInsertElement(Result, LowSum, 2 * i + 0); | ||
| Result = Builder.CreateInsertElement(Result, HighSumPlusCarry, 2 * i + 1, | ||
| "hlsl.AddUint64"); | ||
| } | ||
| return Result; | ||
| } | ||
| case Builtin::BI__builtin_hlsl_resource_getpointer: { | ||
| Value *HandleOp = EmitScalarExpr(E->getArg(0)); | ||
| Value *IndexOp = EmitScalarExpr(E->getArg(1)); | ||
|
|
||
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| // NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --version 5 | ||
| // RUN: %clang_cc1 -finclude-default-header -triple dxil-pc-shadermodel6.3-library %s \ | ||
| // RUN: -emit-llvm -disable-llvm-passes -o - | \ | ||
| // RUN: FileCheck %s --check-prefixes=CHECK | ||
|
|
||
|
|
||
| // CHECK-LABEL: define noundef <2 x i32> @_Z20test_AddUint64_uint2Dv2_jS_( | ||
| // CHECK-SAME: <2 x i32> noundef [[A:%.*]], <2 x i32> noundef [[B:%.*]]) #[[ATTR0:[0-9]+]] { | ||
| // CHECK-NEXT: [[ENTRY:.*:]] | ||
| // CHECK-NEXT: [[A_ADDR:%.*]] = alloca <2 x i32>, align 8 | ||
| // CHECK-NEXT: [[B_ADDR:%.*]] = alloca <2 x i32>, align 8 | ||
| // CHECK-NEXT: store <2 x i32> [[A]], ptr [[A_ADDR]], align 8 | ||
| // CHECK-NEXT: store <2 x i32> [[B]], ptr [[B_ADDR]], align 8 | ||
| // CHECK-NEXT: [[A_LOAD:%.*]] = load <2 x i32>, ptr [[A_ADDR]], align 8 | ||
| // CHECK-NEXT: [[B_LOAD:%.*]] = load <2 x i32>, ptr [[B_ADDR]], align 8 | ||
| // CHECK-NEXT: [[LowA:%.*]] = extractelement <2 x i32> [[A_LOAD]], i64 0 | ||
| // CHECK-NEXT: [[HighA:%.*]] = extractelement <2 x i32> [[A_LOAD]], i64 1 | ||
| // CHECK-NEXT: [[LowB:%.*]] = extractelement <2 x i32> [[B_LOAD]], i64 0 | ||
| // CHECK-NEXT: [[HighB:%.*]] = extractelement <2 x i32> [[B_LOAD]], i64 1 | ||
| // CHECK-NEXT: [[UAddc:%.*]] = call { i32, i1 } @llvm.uadd.with.overflow.i32(i32 [[LowA]], i32 [[LowB]]) | ||
| // CHECK-NEXT: [[Carry:%.*]] = extractvalue { i32, i1 } [[UAddc]], 1 | ||
| // CHECK-NEXT: [[LowSum:%.*]] = extractvalue { i32, i1 } [[UAddc]], 0 | ||
| // CHECK-NEXT: [[CarryZExt:%.*]] = zext i1 [[Carry]] to i32 | ||
| // CHECK-NEXT: [[HighSum:%.*]] = add i32 [[HighA]], [[HighB]] | ||
| // CHECK-NEXT: [[HighSumPlusCarry:%.*]] = add i32 [[HighSum]], [[CarryZExt]] | ||
| // CHECK-NEXT: [[HLSL_ADDUINT64_UPTO0:%.*]] = insertelement <2 x i32> poison, i32 [[LowSum]], i64 0 | ||
| // CHECK-NEXT: [[HLSL_ADDUINT64:%.*]] = insertelement <2 x i32> [[HLSL_ADDUINT64_UPTO0]], i32 [[HighSumPlusCarry]], i64 1 | ||
| // CHECK-NEXT: ret <2 x i32> [[HLSL_ADDUINT64]] | ||
| // | ||
| uint2 test_AddUint64_uint2(uint2 a, uint2 b) { | ||
| return AddUint64(a, b); | ||
| } | ||
|
|
||
| // CHECK-LABEL: define noundef <4 x i32> @_Z20test_AddUint64_uint4Dv4_jS_( | ||
| // CHECK-SAME: <4 x i32> noundef [[A:%.*]], <4 x i32> noundef [[B:%.*]]) #[[ATTR0]] { | ||
| // CHECK-NEXT: [[ENTRY:.*:]] | ||
| // CHECK-NEXT: [[A_ADDR:%.*]] = alloca <4 x i32>, align 16 | ||
| // CHECK-NEXT: [[B_ADDR:%.*]] = alloca <4 x i32>, align 16 | ||
| // CHECK-NEXT: store <4 x i32> [[A]], ptr [[A_ADDR]], align 16 | ||
| // CHECK-NEXT: store <4 x i32> [[B]], ptr [[B_ADDR]], align 16 | ||
| // CHECK-NEXT: [[A_LOAD:%.*]] = load <4 x i32>, ptr [[A_ADDR]], align 16 | ||
| // CHECK-NEXT: [[B_LOAD:%.*]] = load <4 x i32>, ptr [[B_ADDR]], align 16 | ||
| // CHECK-NEXT: [[LowA:%.*]] = extractelement <4 x i32> [[A_LOAD]], i64 0 | ||
| // CHECK-NEXT: [[HighA:%.*]] = extractelement <4 x i32> [[A_LOAD]], i64 1 | ||
| // CHECK-NEXT: [[LowB:%.*]] = extractelement <4 x i32> [[B_LOAD]], i64 0 | ||
| // CHECK-NEXT: [[HighB:%.*]] = extractelement <4 x i32> [[B_LOAD]], i64 1 | ||
| // CHECK-NEXT: [[UAddc:%.*]] = call { i32, i1 } @llvm.uadd.with.overflow.i32(i32 [[LowA]], i32 [[LowB]]) | ||
| // CHECK-NEXT: [[Carry:%.*]] = extractvalue { i32, i1 } [[UAddc]], 1 | ||
| // CHECK-NEXT: [[LowSum:%.*]] = extractvalue { i32, i1 } [[UAddc]], 0 | ||
| // CHECK-NEXT: [[CarryZExt:%.*]] = zext i1 [[Carry]] to i32 | ||
| // CHECK-NEXT: [[HighSum:%.*]] = add i32 [[HighA]], [[HighB]] | ||
| // CHECK-NEXT: [[HighSumPlusCarry:%.*]] = add i32 [[HighSum]], [[CarryZExt]] | ||
| // CHECK-NEXT: [[HLSL_ADDUINT64_UPTO0:%.*]] = insertelement <4 x i32> poison, i32 [[LowSum]], i64 0 | ||
| // CHECK-NEXT: [[HLSL_ADDUINT64_UPTO1:%.*]] = insertelement <4 x i32> [[HLSL_ADDUINT64_UPTO0]], i32 [[HighSumPlusCarry]], i64 1 | ||
| // CHECK-NEXT: [[LowA1:%.*]] = extractelement <4 x i32> [[A_LOAD]], i64 2 | ||
| // CHECK-NEXT: [[HighA1:%.*]] = extractelement <4 x i32> [[A_LOAD]], i64 3 | ||
| // CHECK-NEXT: [[LowB1:%.*]] = extractelement <4 x i32> [[B_LOAD]], i64 2 | ||
| // CHECK-NEXT: [[HighB1:%.*]] = extractelement <4 x i32> [[B_LOAD]], i64 3 | ||
| // CHECK-NEXT: [[UAddc1:%.*]] = call { i32, i1 } @llvm.uadd.with.overflow.i32(i32 [[LowA1]], i32 [[LowB1]]) | ||
| // CHECK-NEXT: [[Carry1:%.*]] = extractvalue { i32, i1 } [[UAddc1]], 1 | ||
| // CHECK-NEXT: [[LowSum1:%.*]] = extractvalue { i32, i1 } [[UAddc1]], 0 | ||
| // CHECK-NEXT: [[CarryZExt1:%.*]] = zext i1 [[Carry1]] to i32 | ||
| // CHECK-NEXT: [[HighSum1:%.*]] = add i32 [[HighA1]], [[HighB1]] | ||
| // CHECK-NEXT: [[HighSumPlusCarry1:%.*]] = add i32 [[HighSum1]], [[CarryZExt1]] | ||
| // CHECK-NEXT: [[HLSL_ADDUINT64_UPTO2:%.*]] = insertelement <4 x i32> [[HLSL_ADDUINT64_UPTO1]], i32 [[LowSum1]], i64 2 | ||
| // CHECK-NEXT: [[HLSL_ADDUINT64:%.*]] = insertelement <4 x i32> [[HLSL_ADDUINT64_UPTO2]], i32 [[HighSumPlusCarry1]], i64 3 | ||
| // CHECK-NEXT: ret <4 x i32> [[HLSL_ADDUINT64]] | ||
| // | ||
| uint4 test_AddUint64_uint4(uint4 a, uint4 b) { | ||
| return AddUint64(a, b); | ||
| } |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| // RUN: %clang_cc1 -finclude-default-header -triple dxil-pc-shadermodel6.6-library %s -fnative-half-type -emit-llvm-only -disable-llvm-passes -verify | ||
|
|
||
| uint2 test_too_few_arg() { | ||
| return __builtin_hlsl_adduint64(); | ||
| // expected-error@-1 {{too few arguments to function call, expected 2, have 0}} | ||
| } | ||
|
|
||
| uint4 test_too_many_arg(uint4 a) { | ||
| return __builtin_hlsl_adduint64(a, a, a); | ||
| // expected-error@-1 {{too many arguments to function call, expected 2, have 3}} | ||
| } | ||
|
|
||
| uint2 test_mismatched_arg_types(uint2 a, uint4 b) { | ||
| return __builtin_hlsl_adduint64(a, b); | ||
| // expected-error@-1 {{all arguments to '__builtin_hlsl_adduint64' must have the same type}} | ||
| } | ||
|
|
||
| uint2 test_bad_num_arg_elements(uint3 a, uint3 b) { | ||
| return __builtin_hlsl_adduint64(a, b); | ||
| // expected-error@-1 {{invalid number of elements in vector operand (expected a uint vector of 2 or 4 elements}} | ||
| } | ||
|
|
||
| uint2 test_scalar_arg_type(uint a) { | ||
| return __builtin_hlsl_adduint64(a, a); | ||
| // expected-error@-1 {{scalar provided to a vector operand (expected a uint vector of 2 or 4 elements)}} | ||
| } | ||
|
|
||
| uint2 test_signed_integer_args(int2 a, int2 b) { | ||
| return __builtin_hlsl_adduint64(a, b); | ||
| // expected-error@-1 {{passing 'int2' (aka 'vector<int, 2>') to parameter of incompatible type '__attribute__((__vector_size__(2 * sizeof(unsigned int)))) unsigned int' (vector of 2 'unsigned int' values)}} | ||
| } | ||
|
|
||
| struct S { | ||
| uint2 a; | ||
| }; | ||
|
|
||
| uint2 test_incorrect_arg_type(S a) { | ||
| return __builtin_hlsl_adduint64(a, a); | ||
| // expected-error@-1 {{passing 'S' to parameter of incompatible type 'unsigned int'}} | ||
| } | ||
|
|
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -230,6 +230,14 @@ static StructType *getSplitDoubleType(LLVMContext &Context) { | |
| return StructType::create({Int32Ty, Int32Ty}, "dx.types.splitdouble"); | ||
| } | ||
|
|
||
| static StructType *getBinaryWithCarryType(LLVMContext &Context) { | ||
|
There was a problem hiding this comment. This is maybe something that we could consider being generated from |
||
| if (auto *ST = StructType::getTypeByName(Context, "dx.types.i32c")) | ||
| return ST; | ||
| Type *Int32Ty = Type::getInt32Ty(Context); | ||
| Type *Int1Ty = Type::getInt1Ty(Context); | ||
| return StructType::create({Int32Ty, Int1Ty}, "dx.types.i32c"); | ||
| } | ||
|
|
||
| static Type *getTypeFromOpParamType(OpParamType Kind, LLVMContext &Ctx, | ||
| Type *OverloadTy) { | ||
| switch (Kind) { | ||
|
|
@@ -273,6 +281,8 @@ static Type *getTypeFromOpParamType(OpParamType Kind, LLVMContext &Ctx, | |
| return getResPropsType(Ctx); | ||
| case OpParamType::SplitDoubleTy: | ||
| return getSplitDoubleType(Ctx); | ||
| case OpParamType::BinaryWithCarryTy: | ||
| return getBinaryWithCarryType(Ctx); | ||
| } | ||
| llvm_unreachable("Invalid parameter kind"); | ||
| return nullptr; | ||
|
|
@@ -539,6 +549,10 @@ StructType *DXILOpBuilder::getSplitDoubleType(LLVMContext &Context) { | |
| return ::getSplitDoubleType(Context); | ||
| } | ||
|
|
||
| StructType *DXILOpBuilder::getBinaryWithCarryType(LLVMContext &Context) { | ||
| return ::getBinaryWithCarryType(Context); | ||
| } | ||
|
|
||
| StructType *DXILOpBuilder::getHandleType() { | ||
| return ::getHandleType(IRB.getContext()); | ||
| } | ||
|
|
||
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
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
Oops, something went wrong.
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.
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.
Few things I think there is already an error for the scalar when expecting a vector case so we should just use that one.
Second this title is too specific to the adduint64 case. we should allow this to be more configurable.
I also don't think this should be hlsl specific. You weren't off base to use
err_vector_incorrect_num_elements. It is correct that the number of elements is wrong but it would be better is if we could specify something likeInvalid element count of 3 when expecting even element counts in the range of 2 to 4. That way we could make 2 and 4 configurable via min max args. Also be nice to make the type configurable too.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 pushed a new commit (eac0fe5).
The case of a scalar being passed as an argument is now back to using
err_vec_builtin_non_vectorto emit an example error message of:all arguments to AddUint64 must be vectorsThe case of a
uint3being passed as an argument is now using a new diag callederr_invalid_even_odd_vector_element_countwhich emits an example error message of:invalid element count of 3 in vector operand (expected an even element count in the range of 2 and 4).I omitted the expected type from the error message, because passing in an invalid type would hit a separate error handler anyways. (The error would be
passing 'int3' (aka 'vector<int, 3>') to parameter of incompatible type '__attribute__((__vector_size__(3 * sizeof(unsigned int)))) unsigned int' (vector of 3 'unsigned int' values)