Addressing partial review comments

Author: raoanag

clang/lib/Headers/hlsl/hlsl_intrinsic_helpers.h

@@ -10,7 +10,7 @@
 #define _HLSL_HLSL_INTRINSIC_HELPERS_H_
 
 namespace hlsl {
-namespace __detail {
+namespace __dETAil {
 
 constexpr vector<uint, 4> d3d_color_to_ubyte4_impl(vector<float, 4> V) {
   // Use the same scaling factor used by FXC, and DXC for DXIL
@@ -71,24 +71,24 @@ constexpr vector<T, L> reflect_vec_impl(vector<T, L> I, vector<T, L> N) {
 #endif
 }
 
-template <typename T> constexpr T refract_impl(T I, T N, T eta) {
-  T k = 1 - eta * eta * (1 - (N * I * N *I));
-  if(k < 0)
+template <typename T> constexpr T refract_impl(T I, T N, T Eta) {
+  T K = 1 - Eta * Eta * (1 - (N * I * N * I));
+  if (K < 0)
     return 0;
   else
-    return (eta * I - (eta * N * I + sqrt(k)) * N);
+    return (Eta * I - (Eta * N * I + sqrt(K)) * N);
 }
 
 template <typename T, int L>
-constexpr vector<T, L> refract_vec_impl(vector<T, L> I, vector<T, L> N, T eta) {
+constexpr vector<T, L> refract_vec_impl(vector<T, L> I, vector<T, L> N, T Eta) {
 #if (__has_builtin(__builtin_spirv_refract))
-  return __builtin_spirv_refract(I, N, eta);
+  return __builtin_spirv_refract(I, N, Eta);
 #else
-  vector<T, L> k = 1 - eta * eta * (1 - dot(N, I) * dot(N, I));
-  if(k < 0)
+  vector<T, L> K = 1 - Eta * Eta * (1 - dot(N, I) * dot(N, I));
+  if (K < 0)
     return 0;
   else
-    return (eta * I - (eta * dot(N, I) + sqrt(k)) * N);
+    return (Eta * I - (Eta * dot(N, I) + sqrt(K)) * N);
 #endif
 }
 

clang/lib/Headers/hlsl/hlsl_intrinsics.h

@@ -487,8 +487,8 @@ reflect(__detail::HLSL_FIXED_VECTOR<float, L> I,
 /// \param eta The refraction index.
 ///
 /// The return value is a floating-point vector that represents the refraction
-/// using the refraction index, \a eta, for the direction of the entering ray, \a I,
-/// off a surface with the normal \a N.
+/// using the refraction index, \a eta, for the direction of the entering ray,
+/// \a I, off a surface with the normal \a N.
 ///
 /// This function calculates the refraction vector using the following formulas:
 /// k = 1.0 - eta * eta * (1.0 - dot(N, I) * dot(N, I))
@@ -515,7 +515,7 @@ const inline __detail::enable_if_t<__detail::is_arithmetic<T>::Value &&
 template <typename T>
 const inline __detail::enable_if_t<
     __detail::is_arithmetic<T>::Value && __detail::is_same<float, T>::value, T>
-    refract(T I, T N, T eta) {
+refract(T I, T N, T eta) {
   return __detail::refract_impl(I, N, eta);
 }
 

clang/lib/Sema/SemaSPIRV.cpp

@@ -173,7 +173,7 @@ bool SemaSPIRV::CheckSPIRVBuiltinFunctionCall(const TargetInfo &TI,
     QualType ArgTyB = B.get()->getType();
     auto *VTyB = ArgTyB->getAs<VectorType>();
     if (VTyB == nullptr) {
-      SemaRef.Diag(A.get()->getBeginLoc(),
+      SemaRef.Diag(B.get()->getBeginLoc(),
                    diag::err_typecheck_convert_incompatible)
           << ArgTyB
           << SemaRef.Context.getVectorType(ArgTyB, 2, VectorKind::Generic) << 1
@@ -234,16 +234,14 @@ bool SemaSPIRV::CheckSPIRVBuiltinFunctionCall(const TargetInfo &TI,
     ExprResult C = TheCall->getArg(2);
     QualType ArgTyC = C.get()->getType();
     if (!ArgTyC->hasFloatingRepresentation()) {
-      SemaRef.Diag(C.get()->getBeginLoc(),
-                   diag::err_builtin_invalid_arg_type)
+      SemaRef.Diag(C.get()->getBeginLoc(), diag::err_builtin_invalid_arg_type)
           << 3 << /* scalar or vector */ 5 << /* no int */ 0 << /* fp */ 1
           << ArgTyC;
       return true;
     }
 
     QualType RetTy = ArgTyA;
     TheCall->setType(RetTy);
-    assert(RetTy == ArgTyA);
     break;
   }
   case SPIRV::BI__builtin_spirv_reflect: {

clang/test/CodeGenSPIRV/Builtins/refract.c

@@ -1,6 +1,6 @@
 // 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
+// RUN: %clang_cc1 -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)));

clang/test/SemaSPIRV/BuiltIns/refract-errors.c

@@ -7,7 +7,7 @@ float2 test_no_second_arg(float2 p0) {
   // expected-error@-1 {{too few arguments to function call, expected 3, have 1}}
 }
 
-float2 test_too_few_arg(float2 p0) {
+float2 test_no_third_arg(float2 p0) {
   return __builtin_spirv_refract(p0, p0);
   // expected-error@-1 {{too few arguments to function call, expected 3, have 2}}
 }