[clang] [llvm] [HLSL] Implement the `faceforward` intrinsic (PR #135878)

Kaitlin Peng via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 18 14:45:34 PDT 2025


https://github.com/kmpeng updated https://github.com/llvm/llvm-project/pull/135878

>From 69aee464d31dcf585c355808053b0c4d1c7d7f3c Mon Sep 17 00:00:00 2001
From: kmpeng <kaitlinpeng at microsoft.com>
Date: Mon, 7 Apr 2025 14:46:07 -0700
Subject: [PATCH 1/4] create int_spv_faceforward intrinsic, create faceforward
 lowering & map to int_spv_faceforward, create SPIR-V backend test case

---
 llvm/include/llvm/IR/IntrinsicsSPIRV.td       |  1 +
 .../Target/SPIRV/SPIRVInstructionSelector.cpp |  2 ++
 .../SPIRV/hlsl-intrinsics/faceforward.ll      | 35 +++++++++++++++++++
 3 files changed, 38 insertions(+)
 create mode 100644 llvm/test/CodeGen/SPIRV/hlsl-intrinsics/faceforward.ll

diff --git a/llvm/include/llvm/IR/IntrinsicsSPIRV.td b/llvm/include/llvm/IR/IntrinsicsSPIRV.td
index 4389b86745d7f..77cca0a58424f 100644
--- a/llvm/include/llvm/IR/IntrinsicsSPIRV.td
+++ b/llvm/include/llvm/IR/IntrinsicsSPIRV.td
@@ -67,6 +67,7 @@ let TargetPrefix = "spv" in {
   def int_spv_cross : DefaultAttrsIntrinsic<[llvm_anyfloat_ty], [LLVMMatchType<0>, LLVMMatchType<0>], [IntrNoMem]>;
   def int_spv_degrees : DefaultAttrsIntrinsic<[LLVMMatchType<0>], [llvm_anyfloat_ty], [IntrNoMem]>;
   def int_spv_distance : DefaultAttrsIntrinsic<[LLVMVectorElementType<0>], [llvm_anyfloat_ty, LLVMMatchType<0>], [IntrNoMem]>;
+  def int_spv_faceforward : DefaultAttrsIntrinsic<[LLVMMatchType<0>], [llvm_anyfloat_ty, LLVMMatchType<0>, LLVMMatchType<0>], [IntrNoMem]>;
   def int_spv_frac : DefaultAttrsIntrinsic<[LLVMMatchType<0>], [llvm_anyfloat_ty], [IntrNoMem]>;
   def int_spv_lerp : DefaultAttrsIntrinsic<[LLVMMatchType<0>], [llvm_anyfloat_ty, LLVMMatchType<0>,LLVMMatchType<0>],
     [IntrNoMem] >;
diff --git a/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp b/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp
index 79f6b43f3aded..8304077f049a3 100644
--- a/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp
@@ -3083,6 +3083,8 @@ bool SPIRVInstructionSelector::selectIntrinsic(Register ResVReg,
     return selectExtInst(ResVReg, ResType, I, CL::length, GL::Length);
   case Intrinsic::spv_degrees:
     return selectExtInst(ResVReg, ResType, I, CL::degrees, GL::Degrees);
+  case Intrinsic::spv_faceforward:
+    return selectExtInst(ResVReg, ResType, I, GL::FaceForward);
   case Intrinsic::spv_frac:
     return selectExtInst(ResVReg, ResType, I, CL::fract, GL::Fract);
   case Intrinsic::spv_normalize:
diff --git a/llvm/test/CodeGen/SPIRV/hlsl-intrinsics/faceforward.ll b/llvm/test/CodeGen/SPIRV/hlsl-intrinsics/faceforward.ll
new file mode 100644
index 0000000000000..4b34adc88c0f2
--- /dev/null
+++ b/llvm/test/CodeGen/SPIRV/hlsl-intrinsics/faceforward.ll
@@ -0,0 +1,35 @@
+; RUN: llc -O0 -verify-machineinstrs -mtriple=spirv-unknown-unknown %s -o - | FileCheck %s
+; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv-unknown-unknown %s -o - -filetype=obj | spirv-val --target-env vulkan1.3 %}
+
+; Make sure SPIRV operation function calls for faceforward are lowered correctly.
+
+; CHECK-DAG: %[[#op_ext_glsl:]] = OpExtInstImport "GLSL.std.450"
+; CHECK-DAG: %[[#float_16:]] = OpTypeFloat 16
+; CHECK-DAG: %[[#vec4_float_16:]] = OpTypeVector %[[#float_16]] 4
+; CHECK-DAG: %[[#float_32:]] = OpTypeFloat 32
+; CHECK-DAG: %[[#vec4_float_32:]] = OpTypeVector %[[#float_32]] 4
+
+define noundef <4 x half> @faceforward_half4(<4 x half> noundef %a, <4 x half> noundef %b, <4 x half> noundef %c) {
+entry:
+  ; CHECK: %[[#]] = OpFunction %[[#vec4_float_16]] None %[[#]]
+  ; CHECK: %[[#arg0:]] = OpFunctionParameter %[[#vec4_float_16]]
+  ; CHECK: %[[#arg1:]] = OpFunctionParameter %[[#vec4_float_16]]
+  ; CHECK: %[[#arg2:]] = OpFunctionParameter %[[#vec4_float_16]]
+  ; CHECK: %[[#]] = OpExtInst %[[#vec4_float_16]] %[[#op_ext_glsl]] FaceForward %[[#arg0]] %[[#arg1]] %[[#arg2]]
+  %spv.faceforward = call <4 x half> @llvm.spv.faceforward.f16(<4 x half> %a, <4 x half> %b, <4 x half> %c)
+  ret <4 x half> %spv.faceforward
+}
+
+define noundef <4 x float> @faceforward_float4(<4 x float> noundef %a, <4 x float> noundef %b, <4 x float> noundef %c) {
+entry:
+  ; CHECK: %[[#]] = OpFunction %[[#vec4_float_32]] None %[[#]]
+  ; CHECK: %[[#arg0:]] = OpFunctionParameter %[[#vec4_float_32]]
+  ; CHECK: %[[#arg1:]] = OpFunctionParameter %[[#vec4_float_32]]
+  ; CHECK: %[[#arg2:]] = OpFunctionParameter %[[#vec4_float_32]]
+  ; CHECK: %[[#]] = OpExtInst %[[#vec4_float_32]] %[[#op_ext_glsl]] FaceForward %[[#arg0]] %[[#arg1]] %[[#arg2]]
+  %spv.faceforward = call <4 x float> @llvm.spv.faceforward.f32(<4 x float> %a, <4 x float> %b, <4 x float> %c)
+  ret <4 x float> %spv.faceforward
+}
+
+declare <4 x half> @llvm.spv.faceforward.f16(<4 x half>, <4 x half>, <4 x half>)
+declare <4 x float> @llvm.spv.faceforward.f32(<4 x float>, <4 x float>, <4 x float>)

>From 06324ca82d3c593c801bf602b8f58683d13e738f Mon Sep 17 00:00:00 2001
From: kmpeng <kaitlinpeng at microsoft.com>
Date: Thu, 10 Apr 2025 14:55:17 -0700
Subject: [PATCH 2/4] implemented `faceforward` in `hlsl.Intrinsics.h` and its
 spir-v target built-in, added codegen and sema tests

---
 clang/include/clang/Basic/BuiltinsSPIRV.td    |  6 ++
 clang/lib/CodeGen/TargetBuiltins/SPIR.cpp     | 12 +++
 .../lib/Headers/hlsl/hlsl_intrinsic_helpers.h | 18 ++++
 clang/lib/Headers/hlsl/hlsl_intrinsics.h      | 47 ++++++++++
 clang/lib/Sema/SemaSPIRV.cpp                  | 36 +++++++
 .../CodeGenHLSL/builtins/faceforward.hlsl     | 94 +++++++++++++++++++
 .../test/CodeGenSPIRV/Builtins/faceforward.c  | 35 +++++++
 .../SemaHLSL/BuiltIns/faceforward-errors.hlsl | 39 ++++++++
 .../SemaSPIRV/BuiltIns/faceforward-errors.c   | 54 +++++++++++
 .../SPIRV/hlsl-intrinsics/faceforward.ll      | 33 ++++++-
 10 files changed, 370 insertions(+), 4 deletions(-)
 create mode 100644 clang/test/CodeGenHLSL/builtins/faceforward.hlsl
 create mode 100644 clang/test/CodeGenSPIRV/Builtins/faceforward.c
 create mode 100644 clang/test/SemaHLSL/BuiltIns/faceforward-errors.hlsl
 create mode 100644 clang/test/SemaSPIRV/BuiltIns/faceforward-errors.c

diff --git a/clang/include/clang/Basic/BuiltinsSPIRV.td b/clang/include/clang/Basic/BuiltinsSPIRV.td
index 9f76d672cc7ce..cc0c2f960f8d2 100644
--- a/clang/include/clang/Basic/BuiltinsSPIRV.td
+++ b/clang/include/clang/Basic/BuiltinsSPIRV.td
@@ -31,3 +31,9 @@ def SPIRVSmoothStep : Builtin {
   let Attributes = [NoThrow, Const, CustomTypeChecking];
   let Prototype = "void(...)";
 }
+
+def SPIRVFaceForward : Builtin {
+  let Spellings = ["__builtin_spirv_faceforward"];
+  let Attributes = [NoThrow, Const, CustomTypeChecking];
+  let Prototype = "void(...)";
+}
diff --git a/clang/lib/CodeGen/TargetBuiltins/SPIR.cpp b/clang/lib/CodeGen/TargetBuiltins/SPIR.cpp
index 92e2c1c6da68f..c1bcc9ae084d0 100644
--- a/clang/lib/CodeGen/TargetBuiltins/SPIR.cpp
+++ b/clang/lib/CodeGen/TargetBuiltins/SPIR.cpp
@@ -71,6 +71,18 @@ Value *CodeGenFunction::EmitSPIRVBuiltinExpr(unsigned BuiltinID,
         ArrayRef<Value *>{Min, Max, X}, /*FMFSource=*/nullptr,
         "spv.smoothstep");
   }
+  case SPIRV::BI__builtin_spirv_faceforward: {
+    Value *N = EmitScalarExpr(E->getArg(0));
+    Value *I = EmitScalarExpr(E->getArg(1));
+    Value *Ng = EmitScalarExpr(E->getArg(2));
+    assert(E->getArg(0)->getType()->hasFloatingRepresentation() &&
+           E->getArg(1)->getType()->hasFloatingRepresentation() &&
+           E->getArg(2)->getType()->hasFloatingRepresentation() &&
+           "FaceForward operands must have a float representation");
+    return Builder.CreateIntrinsic(
+        /*ReturnType=*/N->getType(), Intrinsic::spv_smoothstep,
+        ArrayRef<Value *>{N, I, Ng}, /*FMFSource=*/nullptr, "spv.faceforward");
+  }
   }
   return nullptr;
 }
diff --git a/clang/lib/Headers/hlsl/hlsl_intrinsic_helpers.h b/clang/lib/Headers/hlsl/hlsl_intrinsic_helpers.h
index 3a8a9b6fa2a45..b2332419dc034 100644
--- a/clang/lib/Headers/hlsl/hlsl_intrinsic_helpers.h
+++ b/clang/lib/Headers/hlsl/hlsl_intrinsic_helpers.h
@@ -126,6 +126,24 @@ template <typename T> constexpr vector<T, 4> lit_impl(T NDotL, T NDotH, T M) {
   return Result;
 }
 
+template <typename T> constexpr T faceforward_impl(T N, T I, T Ng) {
+#if (__has_builtin(__builtin_spirv_faceforward))
+  return __builtin_spirv_faceforward(N, I, Ng);
+#else
+  return select(I * Ng < 0, N, -N);
+#endif
+}
+
+template <typename T, int L>
+constexpr vector<T, L> faceforward_vec_impl(vector<T, L> N, vector<T, L> I,
+                                            vector<T, L> Ng) {
+#if (__has_builtin(__builtin_spirv_faceforward))
+  return __builtin_spirv_faceforward(N, I, Ng);
+#else
+  return select(dot(I, Ng) < 0, N, -N);
+#endif
+}
+
 } // namespace __detail
 } // namespace hlsl
 
diff --git a/clang/lib/Headers/hlsl/hlsl_intrinsics.h b/clang/lib/Headers/hlsl/hlsl_intrinsics.h
index 35ff80052cf43..f4af34ea28b86 100644
--- a/clang/lib/Headers/hlsl/hlsl_intrinsics.h
+++ b/clang/lib/Headers/hlsl/hlsl_intrinsics.h
@@ -214,6 +214,53 @@ const inline double4 dst(double4 Src0, double4 Src1) {
   return __detail::dst_impl(Src0, Src1);
 }
 
+//===----------------------------------------------------------------------===//
+// faceforward builtin
+//===----------------------------------------------------------------------===//
+
+/// \fn T faceforward(T N, T I, T Ng)
+/// \brief Flips the surface-normal (if needed) to face in a direction opposite
+/// to \a I. Returns the result in \a N.
+/// \param N The resulting floating-point surface-normal vector.
+/// \param I A floating-point, incident vector that points from the view
+/// position to the shading position.
+/// \param Ng A floating-point surface-normal vector.
+///
+/// Return a floating-point, surface normal vector that is facing the view
+/// direction.
+
+template <typename T>
+_HLSL_16BIT_AVAILABILITY(shadermodel, 6.2)
+const inline __detail::enable_if_t<__detail::is_arithmetic<T>::Value &&
+                                       __detail::is_same<half, T>::value,
+                                   T> faceforward(T N, T I, T Ng) {
+  return __detail::faceforward_impl(N, I, Ng);
+}
+
+template <typename T>
+const inline __detail::enable_if_t<
+    __detail::is_arithmetic<T>::Value && __detail::is_same<float, T>::value, T>
+faceforward(T N, T I, T Ng) {
+  return __detail::faceforward_impl(N, I, Ng);
+}
+
+template <int L>
+_HLSL_16BIT_AVAILABILITY(shadermodel, 6.2)
+const inline __detail::HLSL_FIXED_VECTOR<half, L> faceforward(
+    __detail::HLSL_FIXED_VECTOR<half, L> N,
+    __detail::HLSL_FIXED_VECTOR<half, L> I,
+    __detail::HLSL_FIXED_VECTOR<half, L> Ng) {
+  return __detail::faceforward_vec_impl(N, I, Ng);
+}
+
+template <int L>
+const inline __detail::HLSL_FIXED_VECTOR<float, L>
+faceforward(__detail::HLSL_FIXED_VECTOR<float, L> N,
+            __detail::HLSL_FIXED_VECTOR<float, L> I,
+            __detail::HLSL_FIXED_VECTOR<float, L> Ng) {
+  return __detail::faceforward_vec_impl(N, I, Ng);
+}
+
 //===----------------------------------------------------------------------===//
 // fmod builtins
 //===----------------------------------------------------------------------===//
diff --git a/clang/lib/Sema/SemaSPIRV.cpp b/clang/lib/Sema/SemaSPIRV.cpp
index 7131514d53421..ed40d005c8ce5 100644
--- a/clang/lib/Sema/SemaSPIRV.cpp
+++ b/clang/lib/Sema/SemaSPIRV.cpp
@@ -137,6 +137,42 @@ bool SemaSPIRV::CheckSPIRVBuiltinFunctionCall(unsigned BuiltinID,
     TheCall->setType(RetTy);
     break;
   }
+  case SPIRV::BI__builtin_spirv_faceforward: {
+    if (SemaRef.checkArgCount(TheCall, 3))
+      return true;
+
+    // check if all arguments have floating representation
+    for (unsigned i = 0; i < TheCall->getNumArgs(); ++i) {
+      ExprResult Arg = TheCall->getArg(i);
+      QualType ArgTy = Arg.get()->getType();
+      if (!ArgTy->hasFloatingRepresentation()) {
+        SemaRef.Diag(Arg.get()->getBeginLoc(),
+                     diag::err_builtin_invalid_arg_type)
+            << i + 1 << /* scalar or vector */ 5 << /* no int */ 0 << /* fp */ 1
+            << ArgTy;
+        return true;
+      }
+    }
+
+    // check if all arguments are of the same type
+    ExprResult A = TheCall->getArg(0);
+    ExprResult B = TheCall->getArg(1);
+    ExprResult C = TheCall->getArg(2);
+    if (!(SemaRef.getASTContext().hasSameUnqualifiedType(A.get()->getType(),
+                                                         B.get()->getType()) &&
+          SemaRef.getASTContext().hasSameUnqualifiedType(A.get()->getType(),
+                                                         C.get()->getType()))) {
+      SemaRef.Diag(TheCall->getBeginLoc(),
+                   diag::err_vec_builtin_incompatible_vector)
+          << TheCall->getDirectCallee() << /*useAllTerminology*/ true
+          << SourceRange(A.get()->getBeginLoc(), C.get()->getEndLoc());
+      return true;
+    }
+
+    QualType RetTy = A.get()->getType();
+    TheCall->setType(RetTy);
+    break;
+  }
   }
   return false;
 }
diff --git a/clang/test/CodeGenHLSL/builtins/faceforward.hlsl b/clang/test/CodeGenHLSL/builtins/faceforward.hlsl
new file mode 100644
index 0000000000000..d1942a4bc4b6c
--- /dev/null
+++ b/clang/test/CodeGenHLSL/builtins/faceforward.hlsl
@@ -0,0 +1,94 @@
+// RUN: %clang_cc1 -finclude-default-header -x hlsl -triple \
+// RUN:   dxil-pc-shadermodel6.3-library %s -fnative-half-type \
+// RUN:   -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -finclude-default-header -triple \
+// RUN:   spirv-unknown-vulkan-compute %s -fnative-half-type \
+// RUN:   -emit-llvm -o - | FileCheck %s --check-prefix=SPVCHECK
+
+// CHECK-LABEL: test_faceforward_half
+// CHECK: %mul.i = fmul reassoc nnan ninf nsz arcp afn half %{{.*}}, %{{.*}}
+// CHECK: %cmp.i = fcmp reassoc nnan ninf nsz arcp afn olt half %mul.i, 0xH0000
+// CHECK: %fneg.i = fneg reassoc nnan ninf nsz arcp afn half %{{.*}}
+// CHECK: %hlsl.select.i = select reassoc nnan ninf nsz arcp afn i1 %cmp.i, half %{{.*}}, half %fneg.i
+// CHECK: ret half %hlsl.select.i
+// SPVCHECK-LABEL: test_faceforward_half
+// SPVCHECK: %spv.faceforward.i = call reassoc nnan ninf nsz arcp afn noundef half @llvm.spv.smoothstep.f16(half %{{.*}}, half %{{.*}}, half %{{.*}})
+// SPVCHECK: ret half %spv.faceforward.i
+half test_faceforward_half(half N, half I, half Ng) { return faceforward(N, I, Ng); }
+
+// CHECK-LABEL: test_faceforward_half2
+// CHECK: %hlsl.dot.i = call reassoc nnan ninf nsz arcp afn half @llvm.dx.fdot.v2f16(<2 x half> %{{.*}}, <2 x half> %{{.*}})
+// CHECK: %cmp.i = fcmp reassoc nnan ninf nsz arcp afn olt half %hlsl.dot.i, 0xH0000
+// CHECK: %fneg.i = fneg reassoc nnan ninf nsz arcp afn <2 x half> %{{.*}}
+// CHECK: %hlsl.select.i = select reassoc nnan ninf nsz arcp afn i1 %cmp.i, <2 x half> %{{.*}}, <2 x half> %fneg.i
+// CHECK: ret <2 x half> %hlsl.select.i
+// SPVCHECK-LABEL: test_faceforward_half2
+// SPVCHECK: %spv.faceforward.i = call reassoc nnan ninf nsz arcp afn noundef <2 x half> @llvm.spv.smoothstep.v2f16(<2 x half> %{{.*}}, <2 x half> %{{.*}}, <2 x half> %{{.*}})
+// SPVCHECK: ret <2 x half> %spv.faceforward.i
+half2 test_faceforward_half2(half2 N, half2 I, half2 Ng) { return faceforward(N, I, Ng); }
+
+// CHECK-LABEL: test_faceforward_half3
+// CHECK: %hlsl.dot.i = call reassoc nnan ninf nsz arcp afn half @llvm.dx.fdot.v3f16(<3 x half> %{{.*}}, <3 x half> %{{.*}})
+// CHECK: %cmp.i = fcmp reassoc nnan ninf nsz arcp afn olt half %hlsl.dot.i, 0xH0000
+// CHECK: %fneg.i = fneg reassoc nnan ninf nsz arcp afn <3 x half> %{{.*}}
+// CHECK: %hlsl.select.i = select reassoc nnan ninf nsz arcp afn i1 %cmp.i, <3 x half> %{{.*}}, <3 x half> %fneg.i
+// CHECK: ret <3 x half> %hlsl.select.i
+// SPVCHECK-LABEL: test_faceforward_half3
+// SPVCHECK: %spv.faceforward.i = call reassoc nnan ninf nsz arcp afn noundef <3 x half> @llvm.spv.smoothstep.v3f16(<3 x half> %{{.*}}, <3 x half> %{{.*}}, <3 x half> %{{.*}})
+// SPVCHECK: ret <3 x half> %spv.faceforward.i
+half3 test_faceforward_half3(half3 N, half3 I, half3 Ng) { return faceforward(N, I, Ng); }
+
+// CHECK-LABEL: test_faceforward_half4
+// CHECK: %hlsl.dot.i = call reassoc nnan ninf nsz arcp afn half @llvm.dx.fdot.v4f16(<4 x half> %{{.*}}, <4 x half> %{{.*}})
+// CHECK: %cmp.i = fcmp reassoc nnan ninf nsz arcp afn olt half %hlsl.dot.i, 0xH0000
+// CHECK: %fneg.i = fneg reassoc nnan ninf nsz arcp afn <4 x half> %{{.*}}
+// CHECK: %hlsl.select.i = select reassoc nnan ninf nsz arcp afn i1 %cmp.i, <4 x half> %{{.*}}, <4 x half> %fneg.i
+// CHECK: ret <4 x half> %hlsl.select.i
+// SPVCHECK-LABEL: test_faceforward_half4
+// SPVCHECK: %spv.faceforward.i = call reassoc nnan ninf nsz arcp afn noundef <4 x half> @llvm.spv.smoothstep.v4f16(<4 x half> %{{.*}}, <4 x half> %{{.*}}, <4 x half> %{{.*}})
+// SPVCHECK: ret <4 x half> %spv.faceforward.i
+half4 test_faceforward_half4(half4 N, half4 I, half4 Ng) { return faceforward(N, I, Ng); }
+
+// CHECK-LABEL: test_faceforward_float
+// CHECK: %mul.i = fmul reassoc nnan ninf nsz arcp afn float %{{.*}}, %{{.*}}
+// CHECK: %cmp.i = fcmp reassoc nnan ninf nsz arcp afn olt float %mul.i, 0.000000e+00
+// CHECK: %fneg.i = fneg reassoc nnan ninf nsz arcp afn float %{{.*}}
+// CHECK: %hlsl.select.i = select reassoc nnan ninf nsz arcp afn i1 %cmp.i, float %{{.*}}, float %fneg.i
+// CHECK: ret float %hlsl.select.i
+// SPVCHECK-LABEL: test_faceforward_float
+// SPVCHECK: %spv.faceforward.i = call reassoc nnan ninf nsz arcp afn noundef float @llvm.spv.smoothstep.f32(float %{{.*}}, float %{{.*}}, float %{{.*}})
+// SPVCHECK: ret float %spv.faceforward.i
+float test_faceforward_float(float N, float I, float Ng) { return faceforward(N, I, Ng); }
+
+// CHECK-LABEL: test_faceforward_float2
+// CHECK: %hlsl.dot.i = call reassoc nnan ninf nsz arcp afn float @llvm.dx.fdot.v2f32(<2 x float> %{{.*}}, <2 x float> %{{.*}})
+// CHECK: %cmp.i = fcmp reassoc nnan ninf nsz arcp afn olt float %hlsl.dot.i, 0.000000e+00
+// CHECK: %fneg.i = fneg reassoc nnan ninf nsz arcp afn <2 x float> %{{.*}}
+// CHECK: %hlsl.select.i = select reassoc nnan ninf nsz arcp afn i1 %cmp.i, <2 x float> %{{.*}}, <2 x float> %fneg.i
+// CHECK: ret <2 x float> %hlsl.select.i
+// SPVCHECK-LABEL: test_faceforward_float2
+// SPVCHECK: %spv.faceforward.i = call reassoc nnan ninf nsz arcp afn noundef <2 x float> @llvm.spv.smoothstep.v2f32(<2 x float> %{{.*}}, <2 x float> %{{.*}}, <2 x float> %{{.*}})
+// SPVCHECK: ret <2 x float> %spv.faceforward.i
+float2 test_faceforward_float2(float2 N, float2 I, float2 Ng) { return faceforward(N, I, Ng); }
+
+// CHECK-LABEL: test_faceforward_float3
+// CHECK: %hlsl.dot.i = call reassoc nnan ninf nsz arcp afn float @llvm.dx.fdot.v3f32(<3 x float> %{{.*}}, <3 x float> %{{.*}})
+// CHECK: %cmp.i = fcmp reassoc nnan ninf nsz arcp afn olt float %hlsl.dot.i, 0.000000e+00
+// CHECK: %fneg.i = fneg reassoc nnan ninf nsz arcp afn <3 x float> %{{.*}}
+// CHECK: %hlsl.select.i = select reassoc nnan ninf nsz arcp afn i1 %cmp.i, <3 x float> %{{.*}}, <3 x float> %fneg.i
+// CHECK: ret <3 x float> %hlsl.select.i
+// SPVCHECK-LABEL: test_faceforward_float3
+// SPVCHECK: %spv.faceforward.i = call reassoc nnan ninf nsz arcp afn noundef <3 x float> @llvm.spv.smoothstep.v3f32(<3 x float> %{{.*}}, <3 x float> %{{.*}}, <3 x float> %{{.*}})
+// SPVCHECK: ret <3 x float> %spv.faceforward.i
+float3 test_faceforward_float3(float3 N, float3 I, float3 Ng) { return faceforward(N, I, Ng); }
+
+// CHECK-LABEL: test_faceforward_float4
+// CHECK: %hlsl.dot.i = call reassoc nnan ninf nsz arcp afn float @llvm.dx.fdot.v4f32(<4 x float> %{{.*}}, <4 x float> %{{.*}})
+// CHECK: %cmp.i = fcmp reassoc nnan ninf nsz arcp afn olt float %hlsl.dot.i, 0.000000e+00
+// CHECK: %fneg.i = fneg reassoc nnan ninf nsz arcp afn <4 x float> %{{.*}}
+// CHECK: %hlsl.select.i = select reassoc nnan ninf nsz arcp afn i1 %cmp.i, <4 x float> %{{.*}}, <4 x float> %fneg.i
+// CHECK: ret <4 x float> %hlsl.select.i
+// SPVCHECK-LABEL: test_faceforward_float4
+// SPVCHECK: %spv.faceforward.i = call reassoc nnan ninf nsz arcp afn noundef <4 x float> @llvm.spv.smoothstep.v4f32(<4 x float> %{{.*}}, <4 x float> %{{.*}}, <4 x float> %{{.*}})
+// SPVCHECK: ret <4 x float> %spv.faceforward.i
+float4 test_faceforward_float4(float4 N, float4 I, float4 Ng) { return faceforward(N, I, Ng); }
diff --git a/clang/test/CodeGenSPIRV/Builtins/faceforward.c b/clang/test/CodeGenSPIRV/Builtins/faceforward.c
new file mode 100644
index 0000000000000..17e3268aab9ef
--- /dev/null
+++ b/clang/test/CodeGenSPIRV/Builtins/faceforward.c
@@ -0,0 +1,35 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --version 5
+
+// RUN: %clang_cc1 -O1 -triple spirv-pc-vulkan-compute %s -emit-llvm -o - | FileCheck %s
+
+typedef float float2 __attribute__((ext_vector_type(2)));
+typedef float float3 __attribute__((ext_vector_type(3)));
+typedef float float4 __attribute__((ext_vector_type(4)));
+
+// CHECK-LABEL: define spir_func float @test_faceforward_float(
+// CHECK-SAME: float noundef [[N:%.*]], float noundef [[I:%.*]], float noundef [[NG:%.*]]) local_unnamed_addr #[[ATTR0:[0-9]+]] {
+// CHECK-NEXT:  [[ENTRY:.*:]]
+// CHECK-NEXT:    [[SPV_FACEFORWARD:%.*]] = tail call float @llvm.spv.smoothstep.f32(float [[N]], float [[I]], float [[NG]])
+// CHECK-NEXT:    ret float [[SPV_FACEFORWARD]]
+float test_faceforward_float(float N, float I, float Ng) { return __builtin_spirv_faceforward(N, I, Ng); }
+
+// CHECK-LABEL: define spir_func <2 x float> @test_faceforward_float2(
+// CHECK-SAME: <2 x float> noundef [[N:%.*]], <2 x float> noundef [[I:%.*]], <2 x float> noundef [[NG:%.*]]) local_unnamed_addr #[[ATTR0]] {
+// CHECK-NEXT:  [[ENTRY:.*:]]
+// CHECK-NEXT:    [[SPV_FACEFORWARD:%.*]] = tail call <2 x float> @llvm.spv.smoothstep.v2f32(<2 x float> [[N]], <2 x float> [[I]], <2 x float> [[NG]])
+// CHECK-NEXT:    ret <2 x float> [[SPV_FACEFORWARD]]
+float2 test_faceforward_float2(float2 N, float2 I, float2 Ng) { return __builtin_spirv_faceforward(N, I, Ng); }
+
+// CHECK-LABEL: define spir_func <3 x float> @test_faceforward_float3(
+// CHECK-SAME: <3 x float> noundef [[N:%.*]], <3 x float> noundef [[I:%.*]], <3 x float> noundef [[NG:%.*]]) local_unnamed_addr #[[ATTR0]] {
+// CHECK-NEXT:  [[ENTRY:.*:]]
+// CHECK-NEXT:    [[SPV_FACEFORWARD:%.*]] = tail call <3 x float> @llvm.spv.smoothstep.v3f32(<3 x float> [[N]], <3 x float> [[I]], <3 x float> [[NG]])
+// CHECK-NEXT:    ret <3 x float> [[SPV_FACEFORWARD]]
+float3 test_faceforward_float3(float3 N, float3 I, float3 Ng) { return __builtin_spirv_faceforward(N, I, Ng); }
+
+// CHECK-LABEL: define spir_func <4 x float> @test_faceforward_float4(
+// CHECK-SAME: <4 x float> noundef [[N:%.*]], <4 x float> noundef [[I:%.*]], <4 x float> noundef [[NG:%.*]]) local_unnamed_addr #[[ATTR0]] {
+// CHECK-NEXT:  [[ENTRY:.*:]]
+// CHECK-NEXT:    [[SPV_FACEFORWARD:%.*]] = tail call <4 x float> @llvm.spv.smoothstep.v4f32(<4 x float> [[N]], <4 x float> [[I]], <4 x float> [[NG]])
+// CHECK-NEXT:    ret <4 x float> [[SPV_FACEFORWARD]]
+float4 test_faceforward_float4(float4 N, float4 I, float4 Ng) { return __builtin_spirv_faceforward(N, I, Ng); }
diff --git a/clang/test/SemaHLSL/BuiltIns/faceforward-errors.hlsl b/clang/test/SemaHLSL/BuiltIns/faceforward-errors.hlsl
new file mode 100644
index 0000000000000..469d55995f966
--- /dev/null
+++ b/clang/test/SemaHLSL/BuiltIns/faceforward-errors.hlsl
@@ -0,0 +1,39 @@
+// RUN: %clang_cc1 -finclude-default-header -triple dxil-pc-shadermodel6.6-library %s -fnative-half-type -emit-llvm-only -disable-llvm-passes -verify
+
+float test_double_inputs(double p0, double p1, double p2) {
+  return faceforward(p0, p1, p2);
+  // expected-error at -1  {{no matching function for call to 'faceforward'}}
+  // expected-note at hlsl/hlsl_intrinsics.h:* {{candidate template ignored}}
+  // expected-note at hlsl/hlsl_intrinsics.h:* {{candidate template ignored}}
+  // expected-note at hlsl/hlsl_intrinsics.h:* {{candidate template ignored}}
+  // expected-note at hlsl/hlsl_intrinsics.h:* {{candidate template ignored}}
+}
+
+float test_int_inputs(int p0, int p1, int p2) {
+  return faceforward(p0, p1, p2);
+  // expected-error at -1  {{no matching function for call to 'faceforward'}}
+  // expected-note at hlsl/hlsl_intrinsics.h:* {{candidate template ignored}}
+  // expected-note at hlsl/hlsl_intrinsics.h:* {{candidate template ignored}}
+  // expected-note at hlsl/hlsl_intrinsics.h:* {{candidate template ignored}}
+  // expected-note at hlsl/hlsl_intrinsics.h:* {{candidate template ignored}}
+}
+
+float1 test_vec1_inputs(float1 p0, float1 p1, float1 p2) {
+  return faceforward(p0, p1, p2);
+  // expected-error at -1  {{no matching function for call to 'faceforward'}}
+  // expected-note at hlsl/hlsl_intrinsics.h:* {{candidate template ignored: substitution failure [with T = float1]: no type named 'Type' in 'hlsl::__detail::enable_if<false, vector<float, 1>>'}}
+  // expected-note at hlsl/hlsl_intrinsics.h:* {{candidate template ignored: substitution failure [with T = float1]: no type named 'Type' in 'hlsl::__detail::enable_if<false, vector<float, 1>>'}}
+  // expected-note at hlsl/hlsl_intrinsics.h:* {{candidate template ignored: substitution failure [with L = 1]: no type named 'Type' in 'hlsl::__detail::enable_if<false, half>'}}
+  // expected-note at hlsl/hlsl_intrinsics.h:* {{candidate template ignored: substitution failure [with L = 1]: no type named 'Type' in 'hlsl::__detail::enable_if<false, float>'}}
+}
+
+typedef float float5 __attribute__((ext_vector_type(5)));
+
+float5 test_vec5_inputs(float5 p0, float5 p1, float5 p2) {
+  return faceforward(p0, p1, p2);
+  // expected-error at -1  {{no matching function for call to 'faceforward'}}
+  // expected-note at hlsl/hlsl_intrinsics.h:* {{candidate template ignored: substitution failure [with T = float5]: no type named 'Type' in 'hlsl::__detail::enable_if<false, vector<float, 5>>'}}
+  // expected-note at hlsl/hlsl_intrinsics.h:* {{candidate template ignored: substitution failure [with T = float5]: no type named 'Type' in 'hlsl::__detail::enable_if<false, vector<float, 5>>'}}
+  // expected-note at hlsl/hlsl_intrinsics.h:* {{candidate template ignored: substitution failure [with L = 5]: no type named 'Type' in 'hlsl::__detail::enable_if<false, half>'}}
+  // expected-note at hlsl/hlsl_intrinsics.h:* {{candidate template ignored: substitution failure [with L = 5]: no type named 'Type' in 'hlsl::__detail::enable_if<false, float>'}}
+}
diff --git a/clang/test/SemaSPIRV/BuiltIns/faceforward-errors.c b/clang/test/SemaSPIRV/BuiltIns/faceforward-errors.c
new file mode 100644
index 0000000000000..28089709e6bd5
--- /dev/null
+++ b/clang/test/SemaSPIRV/BuiltIns/faceforward-errors.c
@@ -0,0 +1,54 @@
+// RUN: %clang_cc1 %s -triple spirv-pc-vulkan-compute -verify
+
+typedef float float2 __attribute__((ext_vector_type(2)));
+typedef float float3 __attribute__((ext_vector_type(3)));
+
+float2 test_no_second_arg(float2 p0) {
+  return __builtin_spirv_faceforward(p0);
+  // expected-error at -1 {{too few arguments to function call, expected 3, have 1}}
+}
+
+float2 test_no_third_arg(float2 p0) {
+  return __builtin_spirv_faceforward(p0, p0);
+  // expected-error at -1 {{too few arguments to function call, expected 3, have 2}}
+}
+
+float2 test_too_many_arg(float2 p0) {
+  return __builtin_spirv_faceforward(p0, p0, p0, p0);
+  // expected-error at -1 {{too many arguments to function call, expected 3, have 4}}
+}
+
+int test_int_scalar_inputs(int p0) {
+  return __builtin_spirv_faceforward(p0, p0, p0);
+  //  expected-error at -1 {{1st argument must be a scalar or vector of floating-point types (was 'int')}}
+}
+
+float test_int_scalar_inputs2(float p0, int p1) {
+  return __builtin_spirv_faceforward(p0, p1, p1);
+  //  expected-error at -1 {{2nd argument must be a scalar or vector of floating-point types (was 'int')}}
+}
+
+float test_int_scalar_inputs3(float p0, int p1) {
+  return __builtin_spirv_faceforward(p0, p0, p1);
+  //  expected-error at -1 {{3rd argument must be a scalar or vector of floating-point types (was 'int')}}
+}
+
+float test_mismatched_arg(float p0, float2 p1) {
+  return __builtin_spirv_faceforward(p0, p1, p1);
+  // expected-error at -1 {{all arguments to '__builtin_spirv_faceforward' must have the same type}}
+}
+
+float test_mismatched_arg2(float p0, float2 p1) {
+  return __builtin_spirv_faceforward(p0, p0, p1);
+  // expected-error at -1 {{all arguments to '__builtin_spirv_faceforward' must have the same type}}
+}
+
+float test_mismatched_return(float2 p0) {
+  return __builtin_spirv_faceforward(p0, p0, p0);
+  // expected-error at -1 {{returning 'float2' (vector of 2 'float' values) from a function with incompatible result type 'float'}}
+}
+
+float3 test_mismatched_return2(float2 p0) {
+  return __builtin_spirv_faceforward(p0, p0, p0);
+  // expected-error at -1 {{returning 'float2' (vector of 2 'float' values) from a function with incompatible result type 'float3' (vector of 3 'float' values)}}
+}
diff --git a/llvm/test/CodeGen/SPIRV/hlsl-intrinsics/faceforward.ll b/llvm/test/CodeGen/SPIRV/hlsl-intrinsics/faceforward.ll
index 4b34adc88c0f2..dc1e1a10298e2 100644
--- a/llvm/test/CodeGen/SPIRV/hlsl-intrinsics/faceforward.ll
+++ b/llvm/test/CodeGen/SPIRV/hlsl-intrinsics/faceforward.ll
@@ -9,6 +9,28 @@
 ; CHECK-DAG: %[[#float_32:]] = OpTypeFloat 32
 ; CHECK-DAG: %[[#vec4_float_32:]] = OpTypeVector %[[#float_32]] 4
 
+define noundef half @faceforward_half(half noundef %a, half noundef %b, half noundef %c) {
+entry:
+  ; CHECK: %[[#]] = OpFunction %[[#float_16]] None %[[#]]
+  ; CHECK: %[[#arg0:]] = OpFunctionParameter %[[#float_16]]
+  ; CHECK: %[[#arg1:]] = OpFunctionParameter %[[#float_16]]
+  ; CHECK: %[[#arg2:]] = OpFunctionParameter %[[#float_16]]
+  ; CHECK: %[[#]] = OpExtInst %[[#float_16]] %[[#op_ext_glsl]] FaceForward %[[#arg0]] %[[#arg1]] %[[#arg2]]
+  %spv.faceforward = call half @llvm.spv.faceforward.f16(half %a, half %b, half %c)
+  ret half %spv.faceforward
+}
+
+define noundef float @smoothstep_float(float noundef %a, float noundef %b, float noundef %c) {
+entry:
+  ; CHECK: %[[#]] = OpFunction %[[#float_32]] None %[[#]]
+  ; CHECK: %[[#arg0:]] = OpFunctionParameter %[[#float_32]]
+  ; CHECK: %[[#arg1:]] = OpFunctionParameter %[[#float_32]]
+  ; CHECK: %[[#arg2:]] = OpFunctionParameter %[[#float_32]]
+  ; CHECK: %[[#]] = OpExtInst %[[#float_32]] %[[#op_ext_glsl]] FaceForward %[[#arg0]] %[[#arg1]] %[[#arg2]]
+  %spv.faceforward = call float @llvm.spv.faceforward.f32(float %a, float %b, float %c)
+  ret float %spv.faceforward
+}
+
 define noundef <4 x half> @faceforward_half4(<4 x half> noundef %a, <4 x half> noundef %b, <4 x half> noundef %c) {
 entry:
   ; CHECK: %[[#]] = OpFunction %[[#vec4_float_16]] None %[[#]]
@@ -16,7 +38,7 @@ entry:
   ; CHECK: %[[#arg1:]] = OpFunctionParameter %[[#vec4_float_16]]
   ; CHECK: %[[#arg2:]] = OpFunctionParameter %[[#vec4_float_16]]
   ; CHECK: %[[#]] = OpExtInst %[[#vec4_float_16]] %[[#op_ext_glsl]] FaceForward %[[#arg0]] %[[#arg1]] %[[#arg2]]
-  %spv.faceforward = call <4 x half> @llvm.spv.faceforward.f16(<4 x half> %a, <4 x half> %b, <4 x half> %c)
+  %spv.faceforward = call <4 x half> @llvm.spv.faceforward.v4f16(<4 x half> %a, <4 x half> %b, <4 x half> %c)
   ret <4 x half> %spv.faceforward
 }
 
@@ -27,9 +49,12 @@ entry:
   ; CHECK: %[[#arg1:]] = OpFunctionParameter %[[#vec4_float_32]]
   ; CHECK: %[[#arg2:]] = OpFunctionParameter %[[#vec4_float_32]]
   ; CHECK: %[[#]] = OpExtInst %[[#vec4_float_32]] %[[#op_ext_glsl]] FaceForward %[[#arg0]] %[[#arg1]] %[[#arg2]]
-  %spv.faceforward = call <4 x float> @llvm.spv.faceforward.f32(<4 x float> %a, <4 x float> %b, <4 x float> %c)
+  %spv.faceforward = call <4 x float> @llvm.spv.faceforward.v4f32(<4 x float> %a, <4 x float> %b, <4 x float> %c)
   ret <4 x float> %spv.faceforward
 }
 
-declare <4 x half> @llvm.spv.faceforward.f16(<4 x half>, <4 x half>, <4 x half>)
-declare <4 x float> @llvm.spv.faceforward.f32(<4 x float>, <4 x float>, <4 x float>)
+declare half @llvm.spv.faceforward.f16(half, half, half)
+declare float @llvm.spv.faceforward.f32(float, float, float)
+
+declare <4 x half> @llvm.spv.faceforward.v4f16(<4 x half>, <4 x half>, <4 x half>)
+declare <4 x float> @llvm.spv.faceforward.v4f32(<4 x float>, <4 x float>, <4 x float>)

>From d1a458a0fa5dfbca604d004fb1e6de39deed356a Mon Sep 17 00:00:00 2001
From: kmpeng <kaitlinpeng at microsoft.com>
Date: Wed, 16 Apr 2025 13:44:17 -0700
Subject: [PATCH 3/4] address PR comments - move repeated code to helper
 function, remove -x specification in tests, clarify faceforward desc

---
 clang/lib/Headers/hlsl/hlsl_intrinsics.h      |  2 +-
 clang/lib/Sema/SemaSPIRV.cpp                  | 91 ++++++++-----------
 .../CodeGenHLSL/builtins/faceforward.hlsl     |  2 +-
 3 files changed, 41 insertions(+), 54 deletions(-)

diff --git a/clang/lib/Headers/hlsl/hlsl_intrinsics.h b/clang/lib/Headers/hlsl/hlsl_intrinsics.h
index f4af34ea28b86..4e8fabbed65c4 100644
--- a/clang/lib/Headers/hlsl/hlsl_intrinsics.h
+++ b/clang/lib/Headers/hlsl/hlsl_intrinsics.h
@@ -220,7 +220,7 @@ const inline double4 dst(double4 Src0, double4 Src1) {
 
 /// \fn T faceforward(T N, T I, T Ng)
 /// \brief Flips the surface-normal (if needed) to face in a direction opposite
-/// to \a I. Returns the result in \a N.
+/// to \a I. Returns the result in terms of \a N.
 /// \param N The resulting floating-point surface-normal vector.
 /// \param I A floating-point, incident vector that points from the view
 /// position to the shading position.
diff --git a/clang/lib/Sema/SemaSPIRV.cpp b/clang/lib/Sema/SemaSPIRV.cpp
index ed40d005c8ce5..a008eb02f8581 100644
--- a/clang/lib/Sema/SemaSPIRV.cpp
+++ b/clang/lib/Sema/SemaSPIRV.cpp
@@ -16,6 +16,37 @@ namespace clang {
 
 SemaSPIRV::SemaSPIRV(Sema &S) : SemaBase(S) {}
 
+static bool CheckAllArgsHaveFloatRepresentation(Sema *S, CallExpr *TheCall) {
+  for (unsigned I = 0, N = TheCall->getNumArgs(); I < N; ++I) {
+    ExprResult Arg = TheCall->getArg(I);
+    QualType ArgTy = Arg.get()->getType();
+    if (!ArgTy->hasFloatingRepresentation()) {
+      S->Diag(Arg.get()->getBeginLoc(), diag::err_builtin_invalid_arg_type)
+          << /* ordinal */ I + 1 << /* scalar or vector */ 5 << /* no int */ 0
+          << /* fp */ 1 << ArgTy;
+      return true;
+    }
+  }
+  return false;
+}
+
+static bool CheckAllArgsHaveSameType(Sema *S, CallExpr *TheCall) {
+  assert(TheCall->getNumArgs() > 1);
+  QualType ArgTy0 = TheCall->getArg(0)->getType();
+
+  for (unsigned I = 1, N = TheCall->getNumArgs(); I < N; ++I) {
+    if (!S->getASTContext().hasSameUnqualifiedType(
+            ArgTy0, TheCall->getArg(I)->getType())) {
+      S->Diag(TheCall->getBeginLoc(), diag::err_vec_builtin_incompatible_vector)
+          << TheCall->getDirectCallee() << /*useAllTerminology*/ true
+          << SourceRange(TheCall->getArg(0)->getBeginLoc(),
+                         TheCall->getArg(N - 1)->getEndLoc());
+      return true;
+    }
+  }
+  return false;
+}
+
 bool SemaSPIRV::CheckSPIRVBuiltinFunctionCall(unsigned BuiltinID,
                                               CallExpr *TheCall) {
   switch (BuiltinID) {
@@ -105,35 +136,13 @@ bool SemaSPIRV::CheckSPIRVBuiltinFunctionCall(unsigned BuiltinID,
     if (SemaRef.checkArgCount(TheCall, 3))
       return true;
 
-    // check if the all arguments have floating representation
-    for (unsigned i = 0; i < TheCall->getNumArgs(); ++i) {
-      ExprResult Arg = TheCall->getArg(i);
-      QualType ArgTy = Arg.get()->getType();
-      if (!ArgTy->hasFloatingRepresentation()) {
-        SemaRef.Diag(Arg.get()->getBeginLoc(),
-                     diag::err_builtin_invalid_arg_type)
-            << i + 1 << /* scalar or vector */ 5 << /* no int */ 0 << /* fp */ 1
-            << ArgTy;
-        return true;
-      }
-    }
+    if (CheckAllArgsHaveFloatRepresentation(&SemaRef, TheCall))
+      return true;
 
-    // check if all arguments are of the same type
-    ExprResult A = TheCall->getArg(0);
-    ExprResult B = TheCall->getArg(1);
-    ExprResult C = TheCall->getArg(2);
-    if (!(SemaRef.getASTContext().hasSameUnqualifiedType(A.get()->getType(),
-                                                         B.get()->getType()) &&
-          SemaRef.getASTContext().hasSameUnqualifiedType(A.get()->getType(),
-                                                         C.get()->getType()))) {
-      SemaRef.Diag(TheCall->getBeginLoc(),
-                   diag::err_vec_builtin_incompatible_vector)
-          << TheCall->getDirectCallee() << /*useAllTerminology*/ true
-          << SourceRange(A.get()->getBeginLoc(), C.get()->getEndLoc());
+    if (CheckAllArgsHaveSameType(&SemaRef, TheCall))
       return true;
-    }
 
-    QualType RetTy = A.get()->getType();
+    QualType RetTy = TheCall->getArg(0)->getType();
     TheCall->setType(RetTy);
     break;
   }
@@ -141,35 +150,13 @@ bool SemaSPIRV::CheckSPIRVBuiltinFunctionCall(unsigned BuiltinID,
     if (SemaRef.checkArgCount(TheCall, 3))
       return true;
 
-    // check if all arguments have floating representation
-    for (unsigned i = 0; i < TheCall->getNumArgs(); ++i) {
-      ExprResult Arg = TheCall->getArg(i);
-      QualType ArgTy = Arg.get()->getType();
-      if (!ArgTy->hasFloatingRepresentation()) {
-        SemaRef.Diag(Arg.get()->getBeginLoc(),
-                     diag::err_builtin_invalid_arg_type)
-            << i + 1 << /* scalar or vector */ 5 << /* no int */ 0 << /* fp */ 1
-            << ArgTy;
-        return true;
-      }
-    }
+    if (CheckAllArgsHaveFloatRepresentation(&SemaRef, TheCall))
+      return true;
 
-    // check if all arguments are of the same type
-    ExprResult A = TheCall->getArg(0);
-    ExprResult B = TheCall->getArg(1);
-    ExprResult C = TheCall->getArg(2);
-    if (!(SemaRef.getASTContext().hasSameUnqualifiedType(A.get()->getType(),
-                                                         B.get()->getType()) &&
-          SemaRef.getASTContext().hasSameUnqualifiedType(A.get()->getType(),
-                                                         C.get()->getType()))) {
-      SemaRef.Diag(TheCall->getBeginLoc(),
-                   diag::err_vec_builtin_incompatible_vector)
-          << TheCall->getDirectCallee() << /*useAllTerminology*/ true
-          << SourceRange(A.get()->getBeginLoc(), C.get()->getEndLoc());
+    if (CheckAllArgsHaveSameType(&SemaRef, TheCall))
       return true;
-    }
 
-    QualType RetTy = A.get()->getType();
+    QualType RetTy = TheCall->getArg(0)->getType();
     TheCall->setType(RetTy);
     break;
   }
diff --git a/clang/test/CodeGenHLSL/builtins/faceforward.hlsl b/clang/test/CodeGenHLSL/builtins/faceforward.hlsl
index d1942a4bc4b6c..d55503f138412 100644
--- a/clang/test/CodeGenHLSL/builtins/faceforward.hlsl
+++ b/clang/test/CodeGenHLSL/builtins/faceforward.hlsl
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -finclude-default-header -x hlsl -triple \
+// RUN: %clang_cc1 -finclude-default-header -triple \
 // RUN:   dxil-pc-shadermodel6.3-library %s -fnative-half-type \
 // RUN:   -emit-llvm -o - | FileCheck %s
 // RUN: %clang_cc1 -finclude-default-header -triple \

>From a67d579dc3b020c01e3a17ded82a97c209cf6466 Mon Sep 17 00:00:00 2001
From: kmpeng <kaitlinpeng at microsoft.com>
Date: Fri, 18 Apr 2025 14:45:15 -0700
Subject: [PATCH 4/4] address PR comments - removed faceforward_vec_impl, only
 check first arg is float in sema spirv checks, add half tests to codegen
 spirv test

---
 .../lib/Headers/hlsl/hlsl_intrinsic_helpers.h | 10 ------
 clang/lib/Headers/hlsl/hlsl_intrinsics.h      |  4 +--
 clang/lib/Sema/SemaSPIRV.cpp                  | 36 +++++++++----------
 .../CodeGenHLSL/builtins/faceforward.hlsl     |  8 ++---
 .../test/CodeGenSPIRV/Builtins/faceforward.c  | 34 +++++++++++++++++-
 .../SemaSPIRV/BuiltIns/faceforward-errors.c   |  4 +--
 .../SemaSPIRV/BuiltIns/smoothstep-errors.c    |  4 +--
 .../SPIRV/hlsl-intrinsics/faceforward.ll      |  3 ++
 8 files changed, 64 insertions(+), 39 deletions(-)

diff --git a/clang/lib/Headers/hlsl/hlsl_intrinsic_helpers.h b/clang/lib/Headers/hlsl/hlsl_intrinsic_helpers.h
index b2332419dc034..3180492b7de36 100644
--- a/clang/lib/Headers/hlsl/hlsl_intrinsic_helpers.h
+++ b/clang/lib/Headers/hlsl/hlsl_intrinsic_helpers.h
@@ -127,16 +127,6 @@ template <typename T> constexpr vector<T, 4> lit_impl(T NDotL, T NDotH, T M) {
 }
 
 template <typename T> constexpr T faceforward_impl(T N, T I, T Ng) {
-#if (__has_builtin(__builtin_spirv_faceforward))
-  return __builtin_spirv_faceforward(N, I, Ng);
-#else
-  return select(I * Ng < 0, N, -N);
-#endif
-}
-
-template <typename T, int L>
-constexpr vector<T, L> faceforward_vec_impl(vector<T, L> N, vector<T, L> I,
-                                            vector<T, L> Ng) {
 #if (__has_builtin(__builtin_spirv_faceforward))
   return __builtin_spirv_faceforward(N, I, Ng);
 #else
diff --git a/clang/lib/Headers/hlsl/hlsl_intrinsics.h b/clang/lib/Headers/hlsl/hlsl_intrinsics.h
index 4e8fabbed65c4..193e7e6e99498 100644
--- a/clang/lib/Headers/hlsl/hlsl_intrinsics.h
+++ b/clang/lib/Headers/hlsl/hlsl_intrinsics.h
@@ -250,7 +250,7 @@ const inline __detail::HLSL_FIXED_VECTOR<half, L> faceforward(
     __detail::HLSL_FIXED_VECTOR<half, L> N,
     __detail::HLSL_FIXED_VECTOR<half, L> I,
     __detail::HLSL_FIXED_VECTOR<half, L> Ng) {
-  return __detail::faceforward_vec_impl(N, I, Ng);
+  return __detail::faceforward_impl(N, I, Ng);
 }
 
 template <int L>
@@ -258,7 +258,7 @@ const inline __detail::HLSL_FIXED_VECTOR<float, L>
 faceforward(__detail::HLSL_FIXED_VECTOR<float, L> N,
             __detail::HLSL_FIXED_VECTOR<float, L> I,
             __detail::HLSL_FIXED_VECTOR<float, L> Ng) {
-  return __detail::faceforward_vec_impl(N, I, Ng);
+  return __detail::faceforward_impl(N, I, Ng);
 }
 
 //===----------------------------------------------------------------------===//
diff --git a/clang/lib/Sema/SemaSPIRV.cpp b/clang/lib/Sema/SemaSPIRV.cpp
index a008eb02f8581..90888f1417a9d 100644
--- a/clang/lib/Sema/SemaSPIRV.cpp
+++ b/clang/lib/Sema/SemaSPIRV.cpp
@@ -16,20 +16,6 @@ namespace clang {
 
 SemaSPIRV::SemaSPIRV(Sema &S) : SemaBase(S) {}
 
-static bool CheckAllArgsHaveFloatRepresentation(Sema *S, CallExpr *TheCall) {
-  for (unsigned I = 0, N = TheCall->getNumArgs(); I < N; ++I) {
-    ExprResult Arg = TheCall->getArg(I);
-    QualType ArgTy = Arg.get()->getType();
-    if (!ArgTy->hasFloatingRepresentation()) {
-      S->Diag(Arg.get()->getBeginLoc(), diag::err_builtin_invalid_arg_type)
-          << /* ordinal */ I + 1 << /* scalar or vector */ 5 << /* no int */ 0
-          << /* fp */ 1 << ArgTy;
-      return true;
-    }
-  }
-  return false;
-}
-
 static bool CheckAllArgsHaveSameType(Sema *S, CallExpr *TheCall) {
   assert(TheCall->getNumArgs() > 1);
   QualType ArgTy0 = TheCall->getArg(0)->getType();
@@ -136,13 +122,20 @@ bool SemaSPIRV::CheckSPIRVBuiltinFunctionCall(unsigned BuiltinID,
     if (SemaRef.checkArgCount(TheCall, 3))
       return true;
 
-    if (CheckAllArgsHaveFloatRepresentation(&SemaRef, TheCall))
+    // Check if first argument has floating representation
+    ExprResult A = TheCall->getArg(0);
+    QualType ArgTyA = A.get()->getType();
+    if (!ArgTyA->hasFloatingRepresentation()) {
+      SemaRef.Diag(A.get()->getBeginLoc(), diag::err_builtin_invalid_arg_type)
+          << /* ordinal */ 1 << /* scalar or vector */ 5 << /* no int */ 0
+          << /* fp */ 1 << ArgTyA;
       return true;
+    }
 
     if (CheckAllArgsHaveSameType(&SemaRef, TheCall))
       return true;
 
-    QualType RetTy = TheCall->getArg(0)->getType();
+    QualType RetTy = ArgTyA;
     TheCall->setType(RetTy);
     break;
   }
@@ -150,13 +143,20 @@ bool SemaSPIRV::CheckSPIRVBuiltinFunctionCall(unsigned BuiltinID,
     if (SemaRef.checkArgCount(TheCall, 3))
       return true;
 
-    if (CheckAllArgsHaveFloatRepresentation(&SemaRef, TheCall))
+    // Check if first argument has floating representation
+    ExprResult A = TheCall->getArg(0);
+    QualType ArgTyA = A.get()->getType();
+    if (!ArgTyA->hasFloatingRepresentation()) {
+      SemaRef.Diag(A.get()->getBeginLoc(), diag::err_builtin_invalid_arg_type)
+          << /* ordinal */ 1 << /* scalar or vector */ 5 << /* no int */ 0
+          << /* fp */ 1 << ArgTyA;
       return true;
+    }
 
     if (CheckAllArgsHaveSameType(&SemaRef, TheCall))
       return true;
 
-    QualType RetTy = TheCall->getArg(0)->getType();
+    QualType RetTy = ArgTyA;
     TheCall->setType(RetTy);
     break;
   }
diff --git a/clang/test/CodeGenHLSL/builtins/faceforward.hlsl b/clang/test/CodeGenHLSL/builtins/faceforward.hlsl
index d55503f138412..713e9090f9b26 100644
--- a/clang/test/CodeGenHLSL/builtins/faceforward.hlsl
+++ b/clang/test/CodeGenHLSL/builtins/faceforward.hlsl
@@ -6,8 +6,8 @@
 // RUN:   -emit-llvm -o - | FileCheck %s --check-prefix=SPVCHECK
 
 // CHECK-LABEL: test_faceforward_half
-// CHECK: %mul.i = fmul reassoc nnan ninf nsz arcp afn half %{{.*}}, %{{.*}}
-// CHECK: %cmp.i = fcmp reassoc nnan ninf nsz arcp afn olt half %mul.i, 0xH0000
+// CHECK: %hlsl.dot.i = fmul reassoc nnan ninf nsz arcp afn half %{{.*}}, %{{.*}}
+// CHECK: %cmp.i = fcmp reassoc nnan ninf nsz arcp afn olt half %hlsl.dot.i, 0xH0000
 // CHECK: %fneg.i = fneg reassoc nnan ninf nsz arcp afn half %{{.*}}
 // CHECK: %hlsl.select.i = select reassoc nnan ninf nsz arcp afn i1 %cmp.i, half %{{.*}}, half %fneg.i
 // CHECK: ret half %hlsl.select.i
@@ -50,8 +50,8 @@ half3 test_faceforward_half3(half3 N, half3 I, half3 Ng) { return faceforward(N,
 half4 test_faceforward_half4(half4 N, half4 I, half4 Ng) { return faceforward(N, I, Ng); }
 
 // CHECK-LABEL: test_faceforward_float
-// CHECK: %mul.i = fmul reassoc nnan ninf nsz arcp afn float %{{.*}}, %{{.*}}
-// CHECK: %cmp.i = fcmp reassoc nnan ninf nsz arcp afn olt float %mul.i, 0.000000e+00
+// CHECK: %hlsl.dot.i = fmul reassoc nnan ninf nsz arcp afn float %{{.*}}, %{{.*}}
+// CHECK: %cmp.i = fcmp reassoc nnan ninf nsz arcp afn olt float %hlsl.dot.i, 0.000000e+00
 // CHECK: %fneg.i = fneg reassoc nnan ninf nsz arcp afn float %{{.*}}
 // CHECK: %hlsl.select.i = select reassoc nnan ninf nsz arcp afn i1 %cmp.i, float %{{.*}}, float %fneg.i
 // CHECK: ret float %hlsl.select.i
diff --git a/clang/test/CodeGenSPIRV/Builtins/faceforward.c b/clang/test/CodeGenSPIRV/Builtins/faceforward.c
index 17e3268aab9ef..c5a2f5ee4ff0f 100644
--- a/clang/test/CodeGenSPIRV/Builtins/faceforward.c
+++ b/clang/test/CodeGenSPIRV/Builtins/faceforward.c
@@ -2,12 +2,44 @@
 
 // RUN: %clang_cc1 -O1 -triple spirv-pc-vulkan-compute %s -emit-llvm -o - | FileCheck %s
 
+typedef _Float16 half;
+typedef half half2 __attribute__((ext_vector_type(2)));
+typedef half half3 __attribute__((ext_vector_type(3)));
+typedef half half4 __attribute__((ext_vector_type(4)));
 typedef float float2 __attribute__((ext_vector_type(2)));
 typedef float float3 __attribute__((ext_vector_type(3)));
 typedef float float4 __attribute__((ext_vector_type(4)));
 
+// CHECK-LABEL: define spir_func half @test_faceforward_half(
+// CHECK-SAME: half noundef [[N:%.*]], half noundef [[I:%.*]], half noundef [[NG:%.*]]) local_unnamed_addr #[[ATTR0:[0-9]+]] {
+// CHECK-NEXT:  [[ENTRY:.*:]]
+// CHECK-NEXT:    [[SPV_FACEFORWARD:%.*]] = tail call half @llvm.spv.smoothstep.f16(half [[N]], half [[I]], half [[NG]])
+// CHECK-NEXT:    ret half [[SPV_FACEFORWARD]]
+half test_faceforward_half(half N, half I, half Ng) { return __builtin_spirv_faceforward(N, I, Ng); }
+
+// CHECK-LABEL: define spir_func <2 x half> @test_faceforward_half2(
+// CHECK-SAME: <2 x half> noundef [[N:%.*]], <2 x half> noundef [[I:%.*]], <2 x half> noundef [[NG:%.*]]) local_unnamed_addr #[[ATTR0]] {
+// CHECK-NEXT:  [[ENTRY:.*:]]
+// CHECK-NEXT:    [[SPV_FACEFORWARD:%.*]] = tail call <2 x half> @llvm.spv.smoothstep.v2f16(<2 x half> [[N]], <2 x half> [[I]], <2 x half> [[NG]])
+// CHECK-NEXT:    ret <2 x half> [[SPV_FACEFORWARD]]
+half2 test_faceforward_half2(half2 N, half2 I, half2 Ng) { return __builtin_spirv_faceforward(N, I, Ng); }
+
+// CHECK-LABEL: define spir_func <3 x half> @test_faceforward_half3(
+// CHECK-SAME: <3 x half> noundef [[N:%.*]], <3 x half> noundef [[I:%.*]], <3 x half> noundef [[NG:%.*]]) local_unnamed_addr #[[ATTR0]] {
+// CHECK-NEXT:  [[ENTRY:.*:]]
+// CHECK-NEXT:    [[SPV_FACEFORWARD:%.*]] = tail call <3 x half> @llvm.spv.smoothstep.v3f16(<3 x half> [[N]], <3 x half> [[I]], <3 x half> [[NG]])
+// CHECK-NEXT:    ret <3 x half> [[SPV_FACEFORWARD]]
+half3 test_faceforward_half3(half3 N, half3 I, half3 Ng) { return __builtin_spirv_faceforward(N, I, Ng); }
+
+// CHECK-LABEL: define spir_func <4 x half> @test_faceforward_half4(
+// CHECK-SAME: <4 x half> noundef [[N:%.*]], <4 x half> noundef [[I:%.*]], <4 x half> noundef [[NG:%.*]]) local_unnamed_addr #[[ATTR0]] {
+// CHECK-NEXT:  [[ENTRY:.*:]]
+// CHECK-NEXT:    [[SPV_FACEFORWARD:%.*]] = tail call <4 x half> @llvm.spv.smoothstep.v4f16(<4 x half> [[N]], <4 x half> [[I]], <4 x half> [[NG]])
+// CHECK-NEXT:    ret <4 x half> [[SPV_FACEFORWARD]]
+half4 test_faceforward_half4(half4 N, half4 I, half4 Ng) { return __builtin_spirv_faceforward(N, I, Ng); }
+
 // CHECK-LABEL: define spir_func float @test_faceforward_float(
-// CHECK-SAME: float noundef [[N:%.*]], float noundef [[I:%.*]], float noundef [[NG:%.*]]) local_unnamed_addr #[[ATTR0:[0-9]+]] {
+// CHECK-SAME: float noundef [[N:%.*]], float noundef [[I:%.*]], float noundef [[NG:%.*]]) local_unnamed_addr #[[ATTR0]] {
 // CHECK-NEXT:  [[ENTRY:.*:]]
 // CHECK-NEXT:    [[SPV_FACEFORWARD:%.*]] = tail call float @llvm.spv.smoothstep.f32(float [[N]], float [[I]], float [[NG]])
 // CHECK-NEXT:    ret float [[SPV_FACEFORWARD]]
diff --git a/clang/test/SemaSPIRV/BuiltIns/faceforward-errors.c b/clang/test/SemaSPIRV/BuiltIns/faceforward-errors.c
index 28089709e6bd5..d708df2843fab 100644
--- a/clang/test/SemaSPIRV/BuiltIns/faceforward-errors.c
+++ b/clang/test/SemaSPIRV/BuiltIns/faceforward-errors.c
@@ -25,12 +25,12 @@ int test_int_scalar_inputs(int p0) {
 
 float test_int_scalar_inputs2(float p0, int p1) {
   return __builtin_spirv_faceforward(p0, p1, p1);
-  //  expected-error at -1 {{2nd argument must be a scalar or vector of floating-point types (was 'int')}}
+  //  expected-error at -1 {{all arguments to '__builtin_spirv_faceforward' must have the same type}}
 }
 
 float test_int_scalar_inputs3(float p0, int p1) {
   return __builtin_spirv_faceforward(p0, p0, p1);
-  //  expected-error at -1 {{3rd argument must be a scalar or vector of floating-point types (was 'int')}}
+  //  expected-error at -1 {{all arguments to '__builtin_spirv_faceforward' must have the same type}}
 }
 
 float test_mismatched_arg(float p0, float2 p1) {
diff --git a/clang/test/SemaSPIRV/BuiltIns/smoothstep-errors.c b/clang/test/SemaSPIRV/BuiltIns/smoothstep-errors.c
index 1d57872c393cc..1f21bf3cb8210 100644
--- a/clang/test/SemaSPIRV/BuiltIns/smoothstep-errors.c
+++ b/clang/test/SemaSPIRV/BuiltIns/smoothstep-errors.c
@@ -25,12 +25,12 @@ int test_int_scalar_inputs(int p0) {
 
 float test_int_scalar_inputs2(float p0, int p1) {
   return __builtin_spirv_smoothstep(p0, p1, p1);
-  //  expected-error at -1 {{2nd argument must be a scalar or vector of floating-point types (was 'int')}}
+  //  expected-error at -1 {{all arguments to '__builtin_spirv_smoothstep' must have the same type}}
 }
 
 float test_int_scalar_inputs3(float p0, int p1) {
   return __builtin_spirv_smoothstep(p0, p0, p1);
-  //  expected-error at -1 {{3rd argument must be a scalar or vector of floating-point types (was 'int')}}
+  //  expected-error at -1 {{all arguments to '__builtin_spirv_smoothstep' must have the same type}}
 }
 
 float test_mismatched_arg(float p0, float2 p1) {
diff --git a/llvm/test/CodeGen/SPIRV/hlsl-intrinsics/faceforward.ll b/llvm/test/CodeGen/SPIRV/hlsl-intrinsics/faceforward.ll
index dc1e1a10298e2..11f7258e23874 100644
--- a/llvm/test/CodeGen/SPIRV/hlsl-intrinsics/faceforward.ll
+++ b/llvm/test/CodeGen/SPIRV/hlsl-intrinsics/faceforward.ll
@@ -1,6 +1,9 @@
 ; RUN: llc -O0 -verify-machineinstrs -mtriple=spirv-unknown-unknown %s -o - | FileCheck %s
 ; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv-unknown-unknown %s -o - -filetype=obj | spirv-val --target-env vulkan1.3 %}
 
+; TODO(#136344): This test currently fails when --target-env vulkan1.3 is specified.
+; XFAIL: spirv-tools
+
 ; Make sure SPIRV operation function calls for faceforward are lowered correctly.
 
 ; CHECK-DAG: %[[#op_ext_glsl:]] = OpExtInstImport "GLSL.std.450"



More information about the llvm-commits mailing list