[clang][HLSL] Add sign intrinsic part 4 #108396

tgymnich · 2024-09-12T14:00:05Z

Add handling for unsigned integers to hlsl_elementwise_sign
Use select instead of adding dx and spirv intrinsics for unsigned integers as discussed previously

fixes #70078

Related PRs

cc @farzonl @pow2clk @bob80905 @bogner @llvm-beanz

llvmbot · 2024-09-12T14:04:32Z

@llvm/pr-subscribers-backend-x86
@llvm/pr-subscribers-hlsl
@llvm/pr-subscribers-clang-codegen

@llvm/pr-subscribers-clang

Author: Tim Gymnich (tgymnich)

Changes

Add handling for unsigned integers to hlsl_elementwise_sign
Use select instead of adding dx and spirv intrinsics for unsigned integers as discussed previously

Related PRs

Full diff: https://github.com/llvm/llvm-project/pull/108396.diff

4 Files Affected:

(modified) clang/lib/CodeGen/CGBuiltin.cpp (+11-4)
(modified) clang/lib/Headers/hlsl/hlsl_intrinsics.h (+31)
(modified) clang/lib/Sema/SemaHLSL.cpp (+3-3)
(modified) clang/test/CodeGenHLSL/builtins/sign.hlsl (+63)

diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 9950c06a0b9a6b..9ec966b95f8210 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -18871,18 +18871,25 @@ case Builtin::BI__builtin_hlsl_elementwise_isinf: {
     return EmitRuntimeCall(Intrinsic::getDeclaration(&CGM.getModule(), ID));
   }
   case Builtin::BI__builtin_hlsl_elementwise_sign: {
-    Value *Op0 = EmitScalarExpr(E->getArg(0));
+    auto *Arg0 = E->getArg(0);
+    Value *Op0 = EmitScalarExpr(Arg0);
     llvm::Type *Xty = Op0->getType();
     llvm::Type *retType = llvm::Type::getInt32Ty(this->getLLVMContext());
     if (Xty->isVectorTy()) {
-      auto *XVecTy = E->getArg(0)->getType()->getAs<VectorType>();
+      auto *XVecTy = Arg0->getType()->getAs<VectorType>();
       retType = llvm::VectorType::get(
           retType, ElementCount::getFixed(XVecTy->getNumElements()));
     }
-    assert((E->getArg(0)->getType()->hasFloatingRepresentation() ||
-            E->getArg(0)->getType()->hasSignedIntegerRepresentation()) &&
+    assert((Arg0->getType()->hasFloatingRepresentation() ||
+            Arg0->getType()->hasIntegerRepresentation()) &&
            "sign operand must have a float or int representation");
 
+    if (Arg0->getType()->hasUnsignedIntegerRepresentation()) {
+      Value *Cmp = Builder.CreateICmpEQ(Op0, ConstantInt::get(Xty, 0));
+      return Builder.CreateSelect(Cmp, ConstantInt::get(retType, 0),
+                                  ConstantInt::get(retType, 1), "hlsl.sign");
+    }
+
     return Builder.CreateIntrinsic(
         retType, CGM.getHLSLRuntime().getSignIntrinsic(),
         ArrayRef<Value *>{Op0}, nullptr, "hlsl.sign");
diff --git a/clang/lib/Headers/hlsl/hlsl_intrinsics.h b/clang/lib/Headers/hlsl/hlsl_intrinsics.h
index 7a1edd93984de7..f1758073600b0c 100644
--- a/clang/lib/Headers/hlsl/hlsl_intrinsics.h
+++ b/clang/lib/Headers/hlsl/hlsl_intrinsics.h
@@ -1847,6 +1847,19 @@ int3 sign(int16_t3);
 _HLSL_AVAILABILITY(shadermodel, 6.2)
 _HLSL_BUILTIN_ALIAS(__builtin_hlsl_elementwise_sign)
 int4 sign(int16_t4);
+
+_HLSL_AVAILABILITY(shadermodel, 6.2)
+_HLSL_BUILTIN_ALIAS(__builtin_hlsl_elementwise_sign)
+int sign(uint16_t);
+_HLSL_AVAILABILITY(shadermodel, 6.2)
+_HLSL_BUILTIN_ALIAS(__builtin_hlsl_elementwise_sign)
+int2 sign(uint16_t2);
+_HLSL_AVAILABILITY(shadermodel, 6.2)
+_HLSL_BUILTIN_ALIAS(__builtin_hlsl_elementwise_sign)
+int3 sign(uint16_t3);
+_HLSL_AVAILABILITY(shadermodel, 6.2)
+_HLSL_BUILTIN_ALIAS(__builtin_hlsl_elementwise_sign)
+int4 sign(uint16_t4);
 #endif
 
 _HLSL_16BIT_AVAILABILITY(shadermodel, 6.2)
@@ -1871,6 +1884,15 @@ int3 sign(int3);
 _HLSL_BUILTIN_ALIAS(__builtin_hlsl_elementwise_sign)
 int4 sign(int4);
 
+_HLSL_BUILTIN_ALIAS(__builtin_hlsl_elementwise_sign)
+int sign(uint);
+_HLSL_BUILTIN_ALIAS(__builtin_hlsl_elementwise_sign)
+int2 sign(uint2);
+_HLSL_BUILTIN_ALIAS(__builtin_hlsl_elementwise_sign)
+int3 sign(uint3);
+_HLSL_BUILTIN_ALIAS(__builtin_hlsl_elementwise_sign)
+int4 sign(uint4);
+
 _HLSL_BUILTIN_ALIAS(__builtin_hlsl_elementwise_sign)
 int sign(float);
 _HLSL_BUILTIN_ALIAS(__builtin_hlsl_elementwise_sign)
@@ -1889,6 +1911,15 @@ int3 sign(int64_t3);
 _HLSL_BUILTIN_ALIAS(__builtin_hlsl_elementwise_sign)
 int4 sign(int64_t4);
 
+_HLSL_BUILTIN_ALIAS(__builtin_hlsl_elementwise_sign)
+int sign(uint64_t);
+_HLSL_BUILTIN_ALIAS(__builtin_hlsl_elementwise_sign)
+int2 sign(uint64_t2);
+_HLSL_BUILTIN_ALIAS(__builtin_hlsl_elementwise_sign)
+int3 sign(uint64_t3);
+_HLSL_BUILTIN_ALIAS(__builtin_hlsl_elementwise_sign)
+int4 sign(uint64_t4);
+
 _HLSL_BUILTIN_ALIAS(__builtin_hlsl_elementwise_sign)
 int sign(double);
 _HLSL_BUILTIN_ALIAS(__builtin_hlsl_elementwise_sign)
diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp
index 4e44813fe515ce..e0fdb180ad0358 100644
--- a/clang/lib/Sema/SemaHLSL.cpp
+++ b/clang/lib/Sema/SemaHLSL.cpp
@@ -1518,9 +1518,9 @@ bool CheckNoDoubleVectors(Sema *S, CallExpr *TheCall) {
   return CheckArgsTypesAreCorrect(S, TheCall, S->Context.FloatTy,
                                   checkDoubleVector);
 }
-bool CheckFloatingOrSignedIntRepresentation(Sema *S, CallExpr *TheCall) {
+bool CheckFloatingOrIntRepresentation(Sema *S, CallExpr *TheCall) {
   auto checkAllSignedTypes = [](clang::QualType PassedType) -> bool {
-    return !PassedType->hasSignedIntegerRepresentation() &&
+    return !PassedType->hasIntegerRepresentation() &&
            !PassedType->hasFloatingRepresentation();
   };
   return CheckArgsTypesAreCorrect(S, TheCall, S->Context.IntTy,
@@ -1740,7 +1740,7 @@ bool SemaHLSL::CheckBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall) {
     break;
   }
   case Builtin::BI__builtin_hlsl_elementwise_sign: {
-    if (CheckFloatingOrSignedIntRepresentation(&SemaRef, TheCall))
+    if (CheckFloatingOrIntRepresentation(&SemaRef, TheCall))
       return true;
     if (SemaRef.PrepareBuiltinElementwiseMathOneArgCall(TheCall))
       return true;
diff --git a/clang/test/CodeGenHLSL/builtins/sign.hlsl b/clang/test/CodeGenHLSL/builtins/sign.hlsl
index 4bb239fb009e45..8027d47aed9ab2 100644
--- a/clang/test/CodeGenHLSL/builtins/sign.hlsl
+++ b/clang/test/CodeGenHLSL/builtins/sign.hlsl
@@ -112,6 +112,27 @@ int3 test_sign_int16_t3(int16_t3 p0) { return sign(p0); }
 // NATIVE_HALF: %hlsl.sign = call <4 x i32> @llvm.[[TARGET]].sign.v4i16(
 // NATIVE_HALF: ret <4 x i32> %hlsl.sign
 int4 test_sign_int16_t4(int16_t4 p0) { return sign(p0); }
+
+
+// NATIVE_HALF: define [[FNATTRS]] i32 @
+// NATIVE_HALF: [[CMP:%.*]] = icmp eq i16 [[ARG:%.*]], 0
+// NATIVE_HALF: %hlsl.sign = select i1 [[CMP]], i32 0, i32 1
+int test_sign_uint16_t(uint16_t p0) { return sign(p0); }
+
+// NATIVE_HALF: define [[FNATTRS]] <2 x i32> @
+// NATIVE_HALF: [[CMP:%.*]] = icmp eq <2 x i16> [[ARG:%.*]], zeroinitializer
+// NATIVE_HALF: %hlsl.sign = select <2 x i1> [[CMP]], <2 x i32> zeroinitializer, <2 x i32> <i32 1, i32 1>
+int2 test_sign_uint16_t2(uint16_t2 p0) { return sign(p0); }
+
+// NATIVE_HALF: define [[FNATTRS]] <3 x i32> @
+// NATIVE_HALF: [[CMP:%.*]] = icmp eq <3 x i16> [[ARG:%.*]], zeroinitializer
+// NATIVE_HALF: %hlsl.sign = select <3 x i1> [[CMP]], <3 x i32> zeroinitializer, <3 x i32> <i32 1, i32 1, i32 1>
+int3 test_sign_uint16_t3(uint16_t3 p0) { return sign(p0); }
+
+// NATIVE_HALF: define [[FNATTRS]] <4 x i32> @
+// NATIVE_HALF: [[CMP:%.*]] = icmp eq <4 x i16> [[ARG:%.*]], zeroinitializer
+// NATIVE_HALF: %hlsl.sign = select <4 x i1> [[CMP]], <4 x i32> zeroinitializer, <4 x i32> <i32 1, i32 1, i32 1, i32 1>
+int4 test_sign_uint16_t4(uint16_t4 p0) { return sign(p0); }
 #endif // __HLSL_ENABLE_16_BIT
 
 
@@ -136,6 +157,27 @@ int3 test_sign_int3(int3 p0) { return sign(p0); }
 int4 test_sign_int4(int4 p0) { return sign(p0); }
 
 
+// CHECK: define [[FNATTRS]] i32 @
+// CHECK: [[CMP:%.*]] = icmp eq i32 [[ARG:%.*]], 0
+// CHECK: %hlsl.sign = select i1 [[CMP]], i32 0, i32 1
+int test_sign_uint(uint p0) { return sign(p0); }
+
+// CHECK: define [[FNATTRS]] <2 x i32> @
+// CHECK: [[CMP:%.*]] = icmp eq <2 x i32> [[ARG:%.*]], zeroinitializer
+// CHECK: %hlsl.sign = select <2 x i1> [[CMP]], <2 x i32> zeroinitializer, <2 x i32> <i32 1, i32 1>
+int2 test_sign_uint2(uint2 p0) { return sign(p0); }
+
+// CHECK: define [[FNATTRS]] <3 x i32> @
+// CHECK: [[CMP:%.*]] = icmp eq <3 x i32> [[ARG:%.*]], zeroinitializer
+// CHECK: %hlsl.sign = select <3 x i1> [[CMP]], <3 x i32> zeroinitializer, <3 x i32> <i32 1, i32 1, i32 1>
+int3 test_sign_uint3(uint3 p0) { return sign(p0); }
+
+// CHECK: define [[FNATTRS]] <4 x i32> @
+// CHECK: [[CMP:%.*]] = icmp eq <4 x i32> [[ARG:%.*]], zeroinitializer
+// CHECK: %hlsl.sign = select <4 x i1> [[CMP]], <4 x i32> zeroinitializer, <4 x i32> <i32 1, i32 1, i32 1, i32 1>
+int4 test_sign_uint4(uint4 p0) { return sign(p0); }
+
+
 // CHECK: define [[FNATTRS]] i32 @
 // CHECK: %hlsl.sign = call i32 @llvm.[[TARGET]].sign.i64(
 // CHECK: ret i32 %hlsl.sign
@@ -155,3 +197,24 @@ int3 test_sign_int64_t3(int64_t3 p0) { return sign(p0); }
 // CHECK: %hlsl.sign = call <4 x i32> @llvm.[[TARGET]].sign.v4i64(
 // CHECK: ret <4 x i32> %hlsl.sign
 int4 test_sign_int64_t4(int64_t4 p0) { return sign(p0); }
+
+
+// CHECK: define [[FNATTRS]] i32 @
+// CHECK: [[CMP:%.*]] = icmp eq i64 [[ARG:%.*]], 0
+// CHECK: %hlsl.sign = select i1 [[CMP]], i32 0, i32 1
+int test_sign_int64_t(uint64_t p0) { return sign(p0); }
+
+// CHECK: define [[FNATTRS]] <2 x i32> @
+// CHECK: [[CMP:%.*]] = icmp eq <2 x i64> [[ARG:%.*]], zeroinitializer
+// CHECK: %hlsl.sign = select <2 x i1> [[CMP]], <2 x i32> zeroinitializer, <2 x i32> <i32 1, i32 1>
+int2 test_sign_int64_t2(uint64_t2 p0) { return sign(p0); }
+
+// CHECK: define [[FNATTRS]] <3 x i32> @
+// CHECK: [[CMP:%.*]] = icmp eq <3 x i64> [[ARG:%.*]], zeroinitializer
+// CHECK: %hlsl.sign = select <3 x i1> [[CMP]], <3 x i32> zeroinitializer, <3 x i32> <i32 1, i32 1, i32 1>
+int3 test_sign_int64_t3(uint64_t3 p0) { return sign(p0); }
+
+// CHECK: define [[FNATTRS]] <4 x i32> @
+// CHECK: [[CMP:%.*]] = icmp eq <4 x i64> [[ARG:%.*]], zeroinitializer
+// CHECK: %hlsl.sign = select <4 x i1> [[CMP]], <4 x i32> zeroinitializer, <4 x i32> <i32 1, i32 1, i32 1, i32 1>
+int4 test_sign_int64_t4(uint64_t4 p0) { return sign(p0); }
\ No newline at end of file

farzonl · 2024-10-09T18:40:26Z

clang/test/CodeGenHLSL/builtins/sign.hlsl

+// CHECK: define [[FNATTRS]] <4 x i32> @
+// CHECK: [[CMP:%.*]] = icmp eq <4 x i64> [[ARG:%.*]], zeroinitializer
+// CHECK: %hlsl.sign = select <4 x i1> [[CMP]], <4 x i32> zeroinitializer, <4 x i32> <i32 1, i32 1, i32 1, i32 1>
+int4 test_sign_int64_t4(uint64_t4 p0) { return sign(p0); }


add a new line.

clang/test/CodeGenHLSL/builtins/sign.hlsl

- add handling for unsigned integers to hlsl_elementwise_sign

farzonl · 2024-10-09T19:52:29Z

@tgymnich I think its time for you to ask for commit access. Follow these steps: https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access

tgymnich · 2024-10-09T20:08:33Z

Will do that. I just rebased the PR, so feel free to merge this for me in the meanwhile.

farzonl · 2024-10-10T09:18:04Z

Build failure was on Driver/hip-partial-link.hip Not related to this change. Will proceed with merging.

@farzonl

- Add handling for unsigned integers to hlsl_elementwise_sign - Use `select` instead of adding dx and spirv intrinsics for unsigned integers as [discussed previously ](llvm#101988 (comment)) fixes llvm#70078 ### Related PRs - llvm#101987 - llvm#101988 - llvm#101989 cc @farzonl @pow2clk @bob80905 @bogner @llvm-beanz

bob80905 · 2024-10-16T17:24:05Z

clang/test/CodeGenHLSL/builtins/sign.hlsl

+// CHECK: define [[FNATTRS]] i32 @
+// CHECK: [[CMP:%.*]] = icmp eq i64 [[ARG:%.*]], 0
+// CHECK: %hlsl.sign = select i1 [[CMP]], i32 0, i32 1
+int test_sign_int64_t(uint64_t p0) { return sign(p0); }


The function names are missing a 'u'

https://github.com/llvm/llvm-project/pull/112602/files

@bob80905

Fix test names changing `int` to `uint`. #108396 (comment) @bob80905

@bob80905

Fix test names changing `int` to `uint`. llvm#108396 (comment) @bob80905

@bob80905

Fix test names changing `int` to `uint`. llvm#108396 (comment) @bob80905

tgymnich marked this pull request as ready for review September 12, 2024 14:03

llvmbot added clang Clang issues not falling into any other category backend:X86 clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:headers Headers provided by Clang, e.g. for intrinsics clang:codegen HLSL HLSL Language Support labels Sep 12, 2024

farzonl reviewed Oct 9, 2024

View reviewed changes

farzonl approved these changes Oct 9, 2024

View reviewed changes

tgymnich commented Oct 9, 2024

View reviewed changes

clang/test/CodeGenHLSL/builtins/sign.hlsl Outdated Show resolved Hide resolved

spall approved these changes Oct 9, 2024

View reviewed changes

[clang][HLSL] Add sign intrinsic part 4

74c1953

- add handling for unsigned integers to hlsl_elementwise_sign

tgymnich force-pushed the hlsl-sign-4 branch from 8b09c62 to 74c1953 Compare October 9, 2024 20:08

farzonl merged commit 99608f1 into llvm:main Oct 10, 2024
6 of 8 checks passed

bob80905 reviewed Oct 16, 2024

View reviewed changes

tgymnich mentioned this pull request Oct 16, 2024

[NFC][HLSL] Fix test function names #112602

Merged

bob80905 pushed a commit that referenced this pull request Oct 16, 2024

[NFC][HLSL] Fix test function names (#112602)

2ce0a90

Fix test names changing `int` to `uint`. #108396 (comment) @bob80905

bricknerb pushed a commit to bricknerb/llvm-project that referenced this pull request Oct 17, 2024

[NFC][HLSL] Fix test function names (llvm#112602)

83555c0

Fix test names changing `int` to `uint`. llvm#108396 (comment) @bob80905

EricWF pushed a commit to efcs/llvm-project that referenced this pull request Oct 22, 2024

[NFC][HLSL] Fix test function names (llvm#112602)

33d06a0

Fix test names changing `int` to `uint`. llvm#108396 (comment) @bob80905

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[clang][HLSL] Add sign intrinsic part 4 #108396

[clang][HLSL] Add sign intrinsic part 4 #108396

tgymnich commented Sep 12, 2024 •

edited

Loading

llvmbot commented Sep 12, 2024 •

edited

Loading

Related PRs

farzonl Oct 9, 2024

tgymnich Oct 9, 2024

farzonl commented Oct 9, 2024

tgymnich commented Oct 9, 2024

farzonl commented Oct 10, 2024

bob80905 Oct 16, 2024

tgymnich Oct 16, 2024

[clang][HLSL] Add sign intrinsic part 4 #108396

[clang][HLSL] Add sign intrinsic part 4 #108396

Conversation

tgymnich commented Sep 12, 2024 • edited Loading

Related PRs

llvmbot commented Sep 12, 2024 • edited Loading

Related PRs

farzonl Oct 9, 2024

Choose a reason for hiding this comment

tgymnich Oct 9, 2024

Choose a reason for hiding this comment

farzonl commented Oct 9, 2024

tgymnich commented Oct 9, 2024

farzonl commented Oct 10, 2024

bob80905 Oct 16, 2024

Choose a reason for hiding this comment

tgymnich Oct 16, 2024

Choose a reason for hiding this comment

tgymnich commented Sep 12, 2024 •

edited

Loading

llvmbot commented Sep 12, 2024 •

edited

Loading