[clang] [llvm] [HLSL] Add `Increment`/`DecrementCounter` methods to structured buffers (PR #114148)

Tex Riddell via cfe-commits cfe-commits at lists.llvm.org
Thu Nov 7 16:43:23 PST 2024


================
@@ -343,27 +336,224 @@ struct TemplateParameterListBuilder {
     Params.clear();
 
     QualType T = Builder.Template->getInjectedClassNameSpecialization();
-    T = S.Context.getInjectedClassNameType(Builder.Record, T);
+    T = AST.getInjectedClassNameType(Builder.Record, T);
 
     return Builder;
   }
 };
+
+// Builder for methods of builtin types. Allows adding methods to builtin types
+// using the builder pattern like this:
+//
+//   BuiltinTypeMethodBuilder(Sema, RecordBuilder, "MethodName", ReturnType)
+//       .addParam("param_name", Type, InOutModifier)
+//       .callBuiltin("buildin_name", { BuiltinParams })
+//       .finalizeMethod();
+//
+// The builder needs to have all of the method parameters before it can create
+// a CXXMethodDecl. It collects them in addParam calls and when a first
+// method that builds the body is called it creates the CXXMethodDecl and
+// ParmVarDecls instances. These can then be referenced from the body building
+// methods. Destructor or an explicit call to finalizeMethod() will complete
+// the method definition.
----------------
tex3d wrote:

There are some built-in behaviors/assumptions, some of which probably should be mentioned in the documentation comment.

Here are some oddities I noticed:
- The value returned from the last (statement) expression (builtin call only for now) will be used as the return value for the method (type from builtin must match `ReturnType`).  I get the convenience for this use case, and maybe we never need anything more, but it does seem like we should point this out in the comment.
- It looks like it's trying to be flexible to allow multiple calls to be added, but I don't know any way this architecture would allow you to capture returned values from those calls, reference those in subsequent calls, reference method args as mentioned earlier, etc...  It seems like the interface has been designed tightly around the one use case, but has hints of incompletely designed flexibility (which would likely require changes to this design to take advantage of).
- `callBuiltin()`: Default passing of MemberExpr for the resource handle of `this` from RecordBuilder as the first argument.  I feel like this could be made more explicit, such as by adding a method `BuiltinTypeDeclBuilder::getResourceHandleExpr()`, and a modification of BuiltinParams specified when used below to: `{getResourceHandleExpr(), One}`.

https://github.com/llvm/llvm-project/pull/114148


More information about the cfe-commits mailing list