beanz added a comment.
I have some nitpick comments about the wording of the errors and docs, but the
bigger meta issue is that you've implemented this parsing entirely from
scratch, and I suspect this should be sharing parsing logic with the generic
semantic parser.
I know the parameters themselves have special parsing behavior, but it seems to
me that we could parse this as a normal attribute and verify the arguments post
attribute parsing.
================
Comment at: clang/include/clang/Basic/Attr.td:4018
+ ["SRV", "UAV", "CBuffer", "Sampler"]
+ >, DefaultIntArgument<"ID", 0>,
DefaultIntArgument<"Space", 0>];
+ let Documentation = [HLSLResourceBindingDocs];
----------------
`ID` seems like the wrong name here as it doesn't really convey the meaning of
the value. Maybe `register` or `slot`?
================
Comment at: clang/include/clang/Basic/AttrDocs.td:6516
+ let Content = [{
+The resource binding attribute is to set the virtual registers and logical
register spaces resources in HLSL are bound to.
+Format is ''register(ID, space)'' like register(t3, space1).
----------------
================
Comment at: clang/include/clang/Basic/AttrDocs.td:6517
+The resource binding attribute is to set the virtual registers and logical
register spaces resources in HLSL are bound to.
+Format is ''register(ID, space)'' like register(t3, space1).
+ID must be start with
----------------
================
Comment at: clang/include/clang/Basic/AttrDocs.td:6518
+Format is ''register(ID, space)'' like register(t3, space1).
+ID must be start with
+t for shader resource views (SRV),
----------------
================
Comment at: clang/include/clang/Basic/AttrDocs.td:6524
+
+Register space is default to space0.
+
----------------
================
Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:1607
def err_unknown_hlsl_semantic : Error<"unknown HLSL semantic %0">;
+def err_hlsl_unsupported_register_type : Error<"register type is unsupported -
available types are 'b', 's', 't', 'u'">;
+def err_hlsl_unsupported_register_number : Error<"register number should be an
integral numeric string">;
----------------
================
Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:1608
+def err_hlsl_unsupported_register_type : Error<"register type is unsupported -
available types are 'b', 's', 't', 'u'">;
+def err_hlsl_unsupported_register_number : Error<"register number should be an
integral numeric string">;
+def err_hlsl_expected_space : Error<"expected space definition with syntax
'spaceX', where X is an integral value">;
----------------
================
Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:1609
+def err_hlsl_unsupported_register_number : Error<"register number should be an
integral numeric string">;
+def err_hlsl_expected_space : Error<"expected space definition with syntax
'spaceX', where X is an integral value">;
+def err_hlsl_unsupported_space_number : Error<"space number should be an
integral numeric string">;
----------------
================
Comment at: clang/include/clang/Parse/Parser.h:2821
+
+ void MaybeParseHLSLResourceBinding(ParsedAttributes &Attrs,
+ SourceLocation *EndLoc = nullptr) {
----------------
I don't think this should be fully separate from parsing other HLSL semantics.
================
Comment at: clang/lib/Parse/ParseHLSL.cpp:96
+ auto KwLoc = Tok.getLocation();
+ ConsumeToken(); // consume kw_register.
+
----------------
register isn't a kw.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D130033/new/
https://reviews.llvm.org/D130033
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
