[clang] ff6696c - Expanding HLSL attribute diagnostics

[Chris Bieneman via cfe-commits]

Author: Chris Bieneman
Date: 2022-03-30T14:15:01-05:00
New Revision: ff6696c842bac0b15fc04015b25ead721768eac9

URL: https://github.com/llvm/llvm-project/commit/ff6696c842bac0b15fc04015b25ead721768eac9
DIFF: https://github.com/llvm/llvm-project/commit/ff6696c842bac0b15fc04015b25ead721768eac9.diff

LOG: Expanding HLSL attribute diagnostics

Updating the diagnostics as per the feedback on
https://reviews.llvm.org/D122627.

This change correctly handles missing argument lists, and changes the
subject for the `numthreads` attribute to be global functions.

I did not handle applying the attribute to template functions because
that currently fails parsing in a way that is consisetent with the
current DXC codebase (Microsoft attributes are not supported on
templates).

A future improvement to the diagnostic maybe warranted.

Added: 
    

Modified: 
    clang/include/clang/Basic/Attr.td
    clang/lib/Parse/ParseDeclCXX.cpp
    clang/test/SemaHLSL/num_threads.hlsl

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index 408ea11388c07..97b1027742f60 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -126,6 +126,9 @@ def FunctionTmpl
                                  FunctionDecl::TK_FunctionTemplate}],
                     "function templates">;
 
+def GlobalFunction
+    : SubsetSubject<Function, [{S->isGlobal()}], "global functions">;
+
 def ClassTmpl : SubsetSubject<CXXRecord, [{S->getDescribedClassTemplate()}],
                               "class templates">;
 
@@ -3943,7 +3946,7 @@ def Error : InheritableAttr {
 def HLSLNumThreads: InheritableAttr {
   let Spellings = [Microsoft<"numthreads">];
   let Args = [IntArgument<"X">, IntArgument<"Y">, IntArgument<"Z">];
-  let Subjects = SubjectList<[Function]>;
+  let Subjects = SubjectList<[GlobalFunction]>;
   let LangOpts = [HLSL];
   let Documentation = [NumThreadsDocs];
 }

diff  --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp
index 553dcba94fed6..5d417f5aad1e8 100644
--- a/clang/lib/Parse/ParseDeclCXX.cpp
+++ b/clang/lib/Parse/ParseDeclCXX.cpp
@@ -4701,12 +4701,24 @@ void Parser::ParseMicrosoftAttributes(ParsedAttributes &Attrs) {
         IdentifierInfo *II = Tok.getIdentifierInfo();
         SourceLocation NameLoc = Tok.getLocation();
         ConsumeToken();
-        if (Tok.is(tok::l_paren)) {
-          CachedTokens OpenMPTokens;
-          ParseCXX11AttributeArgs(II, NameLoc, Attrs, &EndLoc, nullptr,
-                                  SourceLocation(), OpenMPTokens);
-          ReplayOpenMPAttributeTokens(OpenMPTokens);
-        } // FIXME: handle attributes that don't have arguments
+        ParsedAttr::Kind AttrKind =
+            ParsedAttr::getParsedKind(II, nullptr, ParsedAttr::AS_Microsoft);
+        // For HLSL we want to handle all attributes, but for MSVC compat, we
+        // silently ignore unknown Microsoft attributes.
+        if (getLangOpts().HLSL || AttrKind != ParsedAttr::UnknownAttribute) {
+          bool AttrParsed = false;
+          if (Tok.is(tok::l_paren)) {
+            CachedTokens OpenMPTokens;
+            AttrParsed =
+                ParseCXX11AttributeArgs(II, NameLoc, Attrs, &EndLoc, nullptr,
+                                        SourceLocation(), OpenMPTokens);
+            ReplayOpenMPAttributeTokens(OpenMPTokens);
+          }
+          if (!AttrParsed) {
+            Attrs.addNew(II, NameLoc, nullptr, SourceLocation(), nullptr, 0,
+                         ParsedAttr::AS_Microsoft);
+          }
+        }
       }
     }
 

diff  --git a/clang/test/SemaHLSL/num_threads.hlsl b/clang/test/SemaHLSL/num_threads.hlsl
index 46e4fa131aa75..cf9e24804a093 100644
--- a/clang/test/SemaHLSL/num_threads.hlsl
+++ b/clang/test/SemaHLSL/num_threads.hlsl
@@ -13,6 +13,12 @@
 #if __SHADER_TARGET_STAGE == __SHADER_STAGE_COMPUTE || __SHADER_TARGET_STAGE == __SHADER_STAGE_MESH || __SHADER_TARGET_STAGE == __SHADER_STAGE_AMPLIFICATION || __SHADER_TARGET_STAGE == __SHADER_STAGE_LIBRARY
 #ifdef FAIL
 #if __SHADER_TARGET_MAJOR == 6
+// expected-error at +1 {{'numthreads' attribute requires exactly 3 arguments}}
+[numthreads]
+// expected-error at +1 {{'numthreads' attribute requires exactly 3 arguments}}
+[numthreads()]
+// expected-error at +1 {{'numthreads' attribute requires exactly 3 arguments}}
+[numthreads(1,2,3,4)]
 // expected-error at +1 {{'numthreads' attribute requires an integer constant}}
 [numthreads("1",2,3)]
 // expected-error at +1 {{argument 'X' to numthreads attribute cannot exceed 1024}}
@@ -38,6 +44,15 @@
 int entry() {
  return 1;
 }
+
+// expected-warning at +1 {{'numthreads' attribute only applies to global functions}}
+[numthreads(1,1,1)]
+struct Fido {
+  // expected-warning at +1 {{'numthreads' attribute only applies to global functions}}
+  [numthreads(1,1,1)]
+  void wag() {}
+};
+
 #else
 // expected-error-re at +1 {{attribute 'numthreads' is unsupported in {{[A-Za-z]+}} shaders, requires Compute, Amplification, Mesh or Library}}
 [numthreads(1,1,1)]