[llvm-branch-commits] [llvm] [HLSLSemanticSignatures] Implement the optimal packing of elements (PR #218064)

[Deric C. via llvm-branch-commits]

================
@@ -499,3 +535,54 @@ Error llvm::hlsl::packSignaturePrefixStable(
 
   return Error::success();
 }
+
+Error llvm::hlsl::packSignatureOptimized(
+    MutableArrayRef<SemanticSignatureElement> Elements,
+    Triple::EnvironmentType ShaderStage, IOType IOTy,
+    bool UseNative16BitTypes) {
+  SmallVector<unsigned> SortedIndices;
+  SortedIndices.reserve(Elements.size());
+  for (unsigned Index = 0; Index != Elements.size(); ++Index)
+    SortedIndices.push_back(Index);
+
+  llvm::sort(SortedIndices, [&](unsigned LeftIndex, unsigned RightIndex) {
+    const SemanticSignatureElement &Left = Elements[LeftIndex];
+    const SemanticSignatureElement &Right = Elements[RightIndex];
+    const unsigned LeftPriority =
+        getOptimizedPackingPriority(Left, ShaderStage, IOTy);
+    const unsigned RightPriority =
+        getOptimizedPackingPriority(Right, ShaderStage, IOTy);
+
+    if (LeftPriority != RightPriority)
+      return LeftPriority < RightPriority;
+    if (Left.InterpMode != Right.InterpMode)
+      return Left.InterpMode < Right.InterpMode;
+    if (Left.Rows != Right.Rows)
+      return Left.Rows > Right.Rows;
+    if (Left.Cols != Right.Cols)
+      return Left.Cols > Right.Cols;
+    return Left.SigId < Right.SigId;
+  });
+
+  // Pack a copy so Elements remains in its original signature order.
+  SmallVector<SemanticSignatureElement> SortedElements;
+  SortedElements.reserve(Elements.size());
+  for (unsigned Index : SortedIndices)
+    SortedElements.push_back(Elements[Index]);
----------------
Icohedron wrote:

[NOTE] Probably not worth the effort to optimize, but it should be noted that this copy does potentially perform a heap allocation for each element because the `SemanticSignatureElement` has a `SmallVector<uint32_t> SemanticIndices` field which is deep-copied. This is also an avoidable expense because only the `StartRow` and `StartCol` fields are mutated past this point.

This includes the `packSignaturePrefixStable` function, which upon brief inspection, appears to only mutate `StartRow` and `StartCol` as well. You may want to redesign the function so that it is clear that only those two fields are mutated.

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