[Sema] Requires nointerpolation on GetAttributeAtVertex param #0

This commit adds new sema checks to restrict the scope of the feature:
the `nointerpolation` attribute is now required on all parameters used
in GetAttributeAtVertex, and doesn't propagage magically to parent/child
functions.

Those issues are mostly caused by the feature which is badly designed
at the language/spec level. The SPIR-V side crashes during validation,
which is a bit unfortunate as the error message is not trivial to
understand. For this reason, I moved some restriction further up.

As for DXIL, we leave it unchanged, as DXIL validation will catch those
cases with a good-enough error message.

The current accepted cases this now blocks were mostly broken later in the
codegen, so this should be an acceptable 'regression'.

Related to #6384, #6220
Fixed #6383, #6382

Signed-off-by: Nathan Gauër <[email protected]>

Author: Keenuts

tools/clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -7732,6 +7732,8 @@ def warn_hlsl_implicit_vector_truncation  : Warning<
   InGroup<Conversion>, DefaultWarn;
 def err_hlsl_nointerpolation_and_linear : Error<
     "nointerpolation cannot be used with any other interpolation mode specifier">;
+def err_hlsl_parameter_requires_attribute : Error<
+    "parameter %0 of %1 must have a a '%2' attribute">;
 def warn_hlsl_duplicate_specifier : Warning<
     "duplicate HLSL specifier %0">,
 	InGroup<IgnoredAttributes>, DefaultWarn;

tools/clang/include/clang/Sema/Sema.h

@@ -8828,6 +8828,8 @@ class Sema {
                         bool AllowOnePastEnd=true, bool IndexNegated=false);
   // HLSL Change Starts - checking array subscript access to vector or matrix member
   void CheckHLSLArrayAccess(const Expr *expr);
+  bool CheckHLSLIntrinsicCall(FunctionDecl *FDecl, CallExpr *TheCall);
+  bool CheckHLSLFunctionCall(FunctionDecl *FDecl, CallExpr *TheCall);
   // HLSL Change ends
   void CheckArrayAccess(const Expr *E);
   // Used to grab the relevant information from a FormatAttr and a

tools/clang/lib/Sema/SemaChecking.cpp

@@ -522,7 +522,6 @@ Sema::CheckBuiltinFunctionCall(FunctionDecl *FDecl, unsigned BuiltinID,
 
     TheCall->setType(Context.VoidPtrTy);
     break;
-
   }
 
   // Since the target specific builtins for each arch overlap, only check those

tools/clang/lib/Sema/SemaExpr.cpp

@@ -5288,7 +5288,10 @@ Sema::BuildResolvedCallExpr(Expr *Fn, NamedDecl *NDecl,
   if (FDecl) {
     if (CheckFunctionCall(FDecl, TheCall, Proto))
       return ExprError();
-
+    if (CheckHLSLFunctionCall(FDecl, TheCall))
+      return ExprError();
+    if (hlsl::IsIntrinsicOp(FDecl) && CheckHLSLIntrinsicCall(FDecl, TheCall))
+      return ExprError();
     if (BuiltinID)
       return CheckBuiltinFunctionCall(FDecl, BuiltinID, TheCall);
   } else if (NDecl) {

tools/clang/lib/Sema/SemaHLSL.cpp

@@ -15176,6 +15176,83 @@ QualType Sema::getHLSLDefaultSpecialization(TemplateDecl *Decl) {
   return QualType();
 }
 
+static bool isRelatedDeclMarkedNointerpolation(Expr *E) {
+  if (!E)
+    return false;
+  E = E->IgnoreCasts();
+  if (auto *DRE = dyn_cast<DeclRefExpr>(E))
+    return DRE->getDecl()->hasAttr<HLSLNoInterpolationAttr>();
+
+  if (auto *ME = dyn_cast<MemberExpr>(E))
+    return ME->getMemberDecl()->hasAttr<HLSLNoInterpolationAttr>() ||
+           isRelatedDeclMarkedNointerpolation(ME->getBase());
+
+  if (auto *HVE = dyn_cast<HLSLVectorElementExpr>(E))
+    return isRelatedDeclMarkedNointerpolation(HVE->getBase());
+
+  if (auto *ASE = dyn_cast<ArraySubscriptExpr>(E))
+    return isRelatedDeclMarkedNointerpolation(ASE->getBase());
+
+  return false;
+}
+
+static bool CheckIntrinsicGetAttributeAtVertex(Sema *S, FunctionDecl *FDecl,
+                                               CallExpr *TheCall) {
+  assert(TheCall->getNumArgs() > 0);
+  auto argument = TheCall->getArg(0)->IgnoreCasts();
+
+  if (!isRelatedDeclMarkedNointerpolation(argument)) {
+    S->Diag(argument->getExprLoc(), diag::err_hlsl_parameter_requires_attribute)
+        << 0 << FDecl->getName() << "nointerpolation";
+    return true;
+  }
+
+  return false;
+}
+
+bool Sema::CheckHLSLIntrinsicCall(FunctionDecl *FDecl, CallExpr *TheCall) {
+  auto attr = FDecl->getAttr<HLSLIntrinsicAttr>();
+
+  switch (hlsl::IntrinsicOp(attr->getOpcode())) {
+  case hlsl::IntrinsicOp::IOP_GetAttributeAtVertex:
+    // See #hlsl-specs/issues/181. Feature is broken. For SPIR-V we want
+    // to limit the scope, and fail gracefully in some cases.
+    if (!getLangOpts().SPIRV)
+      return false;
+    if (FDecl->getName() != "GetAttributeAtVertex")
+      return false;
+    return CheckIntrinsicGetAttributeAtVertex(this, FDecl, TheCall);
+  default:
+    break;
+  }
+
+  return false;
+}
+
+bool Sema::CheckHLSLFunctionCall(FunctionDecl *FDecl, CallExpr *TheCall) {
+  // See #hlsl-specs/issues/181. Feature is broken. For SPIR-V we want
+  // to limit the scope, and fail gracefully in some cases.
+  if (!getLangOpts().SPIRV)
+    return false;
+
+  bool error = false;
+  for (unsigned i = 0; i < FDecl->getNumParams(); i++) {
+    assert(i < TheCall->getNumArgs());
+
+    if (!FDecl->getParamDecl(i)->hasAttr<HLSLNoInterpolationAttr>())
+      continue;
+
+    if (!isRelatedDeclMarkedNointerpolation(TheCall->getArg(i))) {
+      Diag(TheCall->getArg(i)->getExprLoc(),
+           diag::err_hlsl_parameter_requires_attribute)
+          << i << FDecl->getName() << "nointerpolation";
+      error = true;
+    }
+  }
+
+  return error;
+}
+
 namespace hlsl {
 
 static bool nodeInputIsCompatible(DXIL::NodeIOKind IOType,

tools/clang/test/CodeGenSPIRV/intrinsics.get-attribute-at-vertex.s.funcParam.hlsl

@@ -4,7 +4,7 @@ struct S {
   float4 a : COLOR;
 };
 
-float compute(float4 a) {
+float compute(nointerpolation float4 a) {
   return GetAttributeAtVertex(a, 2)[0];
 }
 

tools/clang/test/CodeGenSPIRV/intrinsics.get-attribute-at-vertex.s.funcParam.neg.hlsl

@@ -1,16 +1,17 @@
-// RUN: not %dxc -T ps_6_1 -E main  %s -spirv  2>&1 | FileCheck %s
+// RUN: %dxc -T ps_6_1 -E main  %s -spirv -verify
 
 struct S {
   float4 a : COLOR;
 };
 
-float compute(float4 a) {
+float compute(nointerpolation float4 a) {
   return GetAttributeAtVertex(a, 2)[0];
 }
 
 float4 main(nointerpolation S s, float4 b : COLOR2) : SV_TARGET
 {
-  return float4(0, 0, compute(b), compute(s.a));
+  return float4(0,
+                compute(b), // expected-error{{parameter 0 of compute must have a a 'nointerpolation' attribute}}
+                compute(b), // expected-error{{parameter 0 of compute must have a a 'nointerpolation' attribute}}
+                compute(s.a));
 }
-
-// CHECK: error: Function 'compute' could only use noninterpolated variable as input.

tools/clang/test/SemaHLSL/getattribute-at-vertex.legal.hlsl

@@ -0,0 +1,41 @@
+// RUN: dxc -T ps_6_2 -E main %s -verify
+
+// expected-no-diagnostics
+struct S1 {
+  nointerpolation float3 f1;
+};
+
+struct S2 {
+  float3 f1;
+};
+
+struct S3 {
+  float3 f1[3];
+};
+
+struct S4 {
+  S1 f1;
+};
+
+struct S5 {
+  S1 f1[2];
+};
+
+float4 main(nointerpolation float3 a : A,
+            S1 s1 : B,
+            nointerpolation S2 s2 : C,
+            nointerpolation S3 s3 : D,
+            S4 s4 : E,
+            S5 s5 : F
+            ) : SV_Target
+{
+  float v1 = GetAttributeAtVertex(a, 0)[0];
+  float v2 = GetAttributeAtVertex(a.x, 0);
+  float v3 = GetAttributeAtVertex(s1.f1, 0)[0];
+  float v4 = GetAttributeAtVertex(s2.f1, 0)[0];
+  float v5 = GetAttributeAtVertex(s3.f1[1], 0)[0];
+  float v6 = GetAttributeAtVertex(s4.f1.f1, 0)[0];
+  float v7 = GetAttributeAtVertex(s5.f1[1].f1, 0)[0];
+
+  return float4(v1, v2, v3, v4) + float4(v5, v6, v7.xx);
+}

tools/clang/test/SemaHLSL/getattribute-at-vertex.legal.nointerpolation.call.hlsl

@@ -0,0 +1,16 @@
+// RUN: dxc -T ps_6_2 -E main %s -verify
+
+// expected-no-diagnostics
+float compute(nointerpolation float3 value) {
+  return GetAttributeAtVertex(value, 0)[0];
+}
+
+float middle(nointerpolation float3 value) {
+  return compute(value);
+}
+
+float4 main(nointerpolation float3 a : A) : SV_Target
+{
+  float v1 = middle(a);
+  return float4(v1.xxxx);
+}

tools/clang/test/SemaHLSL/getattribute-at-vertex.missing.nointerpolation.call.dxil.hlsl

@@ -0,0 +1,12 @@
+// RUN: not %dxc -T ps_6_2 -E main %s 2>&1 | FileCheck %s
+
+float compute(nointerpolation float3 value) {
+  return GetAttributeAtVertex(value, 0)[0];
+}
+
+float4 main(float3 a : A) : SV_Target
+{
+  // CHECK: error: Attribute A must have nointerpolation mode in order to use GetAttributeAtVertex function.
+  float v1 = compute(a);
+  return float4(v1.xxxx);
+}