[SPIR-V][Sema] Requires nointerpolation on GetAttributeAtVertex param #0 (#6453)

This commit adds new sema checks for SPIR-V 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.

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

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

---------

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

Author: Keenuts

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

@@ -7735,6 +7735,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 '%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,8 @@ Sema::BuildResolvedCallExpr(Expr *Fn, NamedDecl *NDecl,
   if (FDecl) {
     if (CheckFunctionCall(FDecl, TheCall, Proto))
       return ExprError();
-
+    if (CheckHLSLFunctionCall(FDecl, TheCall))
+      return ExprError();
     if (BuiltinID)
       return CheckBuiltinFunctionCall(FDecl, BuiltinID, TheCall);
   } else if (NDecl) {

tools/clang/lib/Sema/SemaHLSL.cpp

@@ -15184,6 +15184,88 @@ 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;
+    // This should never happen for SPIR-V. But on the DXIL side, extension can
+    // be added by inserting new intrinsics, meaning opcodes can collide with
+    // existing ones. See the ExtensionTest.EvalAttributeCollision test.
+    assert(FDecl->getName() == "GetAttributeAtVertex");
+    return CheckIntrinsicGetAttributeAtVertex(this, FDecl, TheCall);
+  default:
+    break;
+  }
+
+  return false;
+}
+
+bool Sema::CheckHLSLFunctionCall(FunctionDecl *FDecl, CallExpr *TheCall) {
+  if (hlsl::IsIntrinsicOp(FDecl) && CheckHLSLIntrinsicCall(FDecl, TheCall))
+    return true;
+
+  // 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.cbuf.var.hlsl

@@ -1,4 +1,4 @@
-// RUN: dxc -T ps_6_2 -E main -spirv %s | FileCheck %s
+// RUN: %dxc -T ps_6_2 -E main -spirv %s | FileCheck %s
 
 // CHECK:                                       OpDecorate %in_var_A PerVertexKHR
 // CHECK-DAG:                 %type_constants = OpTypeStruct %uint

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 'nointerpolation' attribute}}
+                compute(b), // expected-error{{parameter 0 of compute must have 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,46 @@
+// 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];
+};
+
+float compute(float3 value) {
+  return GetAttributeAtVertex(value, 0)[0];
+}
+
+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];
+  float v8 = compute(s1.f1);
+
+  return float4(v1, v2, v3, v4) + float4(v5, v6, v7, v8);
+}

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);
+}