aaron.ballman added inline comments.

================
Comment at: clang/include/clang/AST/Attr.h:193
 
+class HLSLAnnotationAttr : public InheritableAttr {
+protected:
----------------
beanz wrote:
> aaron.ballman wrote:
> > Is this intended to be used only for parameters (that's how I read the 
> > summary for the patch)? If so, why is this not inheriting from 
> > `InheritableParamAttr`?
> Sadly these attributes are valid in places that aren’t parameters. For 
> example, they can be applied on the members of a struct. When specifying an 
> HLSL entry we require semantics for all scalar variables in the signature so 
> that the we can match up the parameters from driver and user-provided values, 
> but the parameters can be scalars or user defined structs. For example:
> 
> ```
> struct Pupper {
>   int Legs : SomeAnnotation;
>   int Ears : SomeOtherAnnotation;
> }
> ...
> void main(Pupper P) {...} // valid as an entry
> ```
> 
> We also allow these annotations (and require them) on return types for entry 
> functions:
> 
> ```
> int main(...) : SomeAnnotation {...}
> ```
> 
> Where this all gets really special is that entry functions as passed to 
> drivers have a `void(void)` signature, but entry functions with the 
> source-specified signature can be called.
> 
> I'm trying to handle those cases in D131203 by generating the user-written 
> function as is with C++ mangling, and generating a non-mangled `void(void)` 
> wrapper that calls the underlying function after populating the annotation 
> values. It is an incomplete implementation, but a starting point.
> 
Oh, thank you for the explanation!

> Where this all gets really special is that entry functions as passed to 
> drivers have a void(void) signature, but entry functions with the 
> source-specified signature can be called.

Well that should be fun if you have code that cares about the address of the 
function, depending on which address you happen to get. But you don't allow 
pointers IIRC, so maybe you're "safe"?


================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:11635
 def err_hlsl_attribute_param_mismatch : Error<"%0 attribute parameters do not 
match the previous declaration">;
+def err_hlsl_missing_parameter_annotation : Error<"entry function parameter %0 
missing annotation">;
 
----------------
beanz wrote:
> aaron.ballman wrote:
> > Will users know how to fix the issue when they get this diagnostic? 
> > "missing annotation" sounds like "slap any old annotation on there, it'll 
> > be fine".
> I should probably make this closer to our existing diagnostic: "Semantic must 
> be defined for all parameters of an entry function or patch constant 
> function."
That sounds more appropriately descriptive.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131625

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to