Keenuts wrote: > I'm approving this, but think the approach will need structural changes to > handle the general struct case.
Yes, especially with the struct reuse case. As per the last meeting discussion, merging this as-is as it is an improvement over the status quo, but it does require significant additional work before we can call this done. > Specifically: > > I think the attribute added to the function decl to describe a loaded/stored > element should be a different attribute than the semantic attribute. It's a > shader I/O element, aka. signature element, constructed from a path through > parameter space with a semantic applied. > > Another issue is that the user adds an explicit semantic to the function decl > which gets applied to the return value, so adding more copies of semantics > for other things on the same decl when traversing parameters and structs in > parameters makes it hard to differentiate an explicit semantic for the return > value from semantics inferred from interpreting parameters and fields in > types. > > I don't think pointing at the leaf field in a type is enough. You'd need the > full path through the decls that reaches particular instance of the leaf > field, since that type could be used multiple times in the shader > inputs/outputs/return type, and containing types. Recording a full path of > decls from the param to the field is one approach, but I don't think it's > necessarily the right one. > > I think a different approach for storing interpreted signature elements would > be better. We could instead add signature element attributes to each > parameter decl or to the function for the return type, in order of traversal > to leaf fields. These would form a sort of worklist of items to generate code > for loading or storing as you traverse each parameter type. Once one is > processed, it's only looking at the next (field) that applies to the same > param decl or return type. There's no need to search all elements for each > field. You're just matching the next leaf field with the next signature > element and asserting that certain captured properties are consistent with > those properties picked up during this traversal (like the dimensionality and > type). > > This would be different from the current approach which traverses the > parameters, recurses into fields, then for each field, iterates all semantics > on the function decl looking for one with TargetDecl pointing to the current > field. While adding the full path could fix the problems with multiple > instances, it's more complicated and unnecessary if you take the approach I > suggested instead, where we know the next field must match the next element, > no searching necessary. > > Technically, we don't need to store anything more to the AST than what was > explicitly declared in HLSL, as long as we reconstruct the same elements that > were constructed and diagnosed in Sema in exactly the same way. We could do > this if we had shared code for traversal and element construction from the > AST. > > Each parameter or return type, based on shader kind and parameter > modifier/type/location slots into one "Signature Point" (SigPoint for short) > that identifies a grouping of shader I/O parameters handled in a particular > way (more than just input/output, it accounts for things like separating > per-invocation system values from sets of input vertices (GS/HS/DS), or patch > constant outputs (HS) or inputs (DS), and separating per-primitive mesh > shader outputs from per-vertex outputs. > > Whether stored back to the AST, or temporarily constructed for Sema > diagnostics and again for CodeGen, signature elements need some properties > for the DirectX target at least: > > * shape (rows, cols, HLSL component type) > * system value index expansion (one index per row) > > * Full type traversal from decl with active semantic is necessary to assign > these, when this decl is (optionally an array of) a struct type, since other > fields will consume indices from the same set in traversal order. > * which signature this belongs to (and SigPoint), if any > * system value interpretation (user or specific system value) > * interpolation mode > * packing location if element type (by interpretation) is packed into a > signature > Diagnostics will need to keep track of some things that CodeGen doesn't > need: > * per element: decl location with applicable user-specified semantic > attribute for semantics inherited by contained fields > * per sigpoint: map of previously defined elements using semantic name/index > key for diagnosing duplicate/overlapping semantics https://github.com/llvm/llvm-project/pull/159047 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
