[PATCH] D122699: [HLSL] Add Semantic syntax, and SV_GroupIndex

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

You should also add parsing test coverage as this is parsing new syntax.



================
Comment at: clang/include/clang/Basic/AttrDocs.td:6386
+  let Content = [{
+The `SV_GroupIndex` semantic when applied to an input parameter specifies a data
+binding to map the group index to the specified parameter. This attribute is
----------------
MaskRay wrote:
> 



================
Comment at: clang/include/clang/Parse/Parser.h:2785-2792
+  void MaybeParseHLSLSemantics(ParsedAttributes &attrs,
+                               SourceLocation *endLoc = nullptr) {
+    if (getLangOpts().HLSL && Tok.is(tok::colon))
+      ParseHLSLSemantics(attrs, endLoc);
+  }
+
+  void ParseHLSLSemantics(ParsedAttributes &attrs,
----------------



================
Comment at: clang/lib/Parse/ParseDecl.cpp:6965-6966
     // Parse GNU attributes, if present.
     MaybeParseGNUAttributes(ParmDeclarator);
+    MaybeParseHLSLSemantics(DS.getAttributes());
 
----------------
This means the order is very specific -- should this be using `MaybeParseAttributes()` so that the syntaxes can be intermingled?

This reminds me, I don't see a change to `ParseAttrKindMask`, but perhaps we want that for both the microsoft and HLSL semantic attributes?


================
Comment at: clang/lib/Parse/ParseHLSL.cpp:30-33
+  if (AttrKind == ParsedAttr::UnknownAttribute ||
+      AttrKind == ParsedAttr::IgnoredAttribute) {
+    ConsumeToken();
+    return;
----------------
I think we should issue a diagnostic about ignoring an unused attribute here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122699/new/

https://reviews.llvm.org/D122699