Address comments from HanHan

Note, I deleted `getUnPackInverseSrcPerm(UnPackOp unpackOp)` - I
couldn't find any uses of that hook.

Author: banach-space

mlir/include/mlir/Dialect/Linalg/Utils/Utils.h

@@ -33,23 +33,13 @@ namespace linalg {
 //===----------------------------------------------------------------------===//
 // Utilities for inferring various semantics properties of Linalg ops.
 //===----------------------------------------------------------------------===//
-/// Shell function to compute the Destination Permutation of PackOp
-/// This function uses the helper function `computePackUnPackPerm` to get
-/// the permutation vector. Only major difference between UnPack and Pack is
-/// that packOp uses destination rank whereas unpack Uses source rank.
+/// Compute inverse permutation for the destination tensor (i.e. in the packed
+/// domain).
 SmallVector<int64_t> getPackInverseDestPerm(linalg::PackOp packOp,
-                                            PackingMetadata &metadatap);
-
-/// Shell function to compute the Source Permutation of unPackOp.
-/// This function, like the getPackInverseDestPerm uses the helper function
-/// computePackUnPackPerm` to get the permutation vector.
-/// Only major difference between UnPack and Pack is that packOp uses
-/// destination rank whereas unpack Uses source rank.
-SmallVector<int64_t> getUnPackInverseSrcPerm(linalg::UnPackOp unpackOp);
-
-/// Shell function to compute the Source rank permutation for unpackOp
-/// Unpack requires some packing metadata data information, so created
-/// another function where this value is passed by reference.
+                                            PackingMetadata &metadata);
+
+/// Compute inverse permutation for the source tensor (i.e. in the packed
+/// domain).
 SmallVector<int64_t> getUnPackInverseSrcPerm(linalg::UnPackOp,
                                              PackingMetadata &metadata);
 

mlir/lib/Dialect/Linalg/Transforms/Vectorization.cpp

@@ -1861,7 +1861,7 @@ vectorizeAsTensorPackOp(RewriterBase &rewriter, linalg::PackOp packOp,
   if (writeVectorSizes.empty()) {
     if (ShapedType::isDynamicShape(destShape))
       return rewriter.notifyMatchFailure(packOp,
-                                         "Unable to infer vector sizes!");
+                                         "unable to infer vector sizes");
 
     writeVectorSizes = destShape;
     useInBoundsInsteadOfMasking = true;

mlir/lib/Dialect/Linalg/Utils/Utils.cpp

@@ -172,21 +172,16 @@ namespace mlir {
 namespace linalg {
 
 SmallVector<int64_t> getPackInverseDestPerm(PackOp packOp,
-                                            PackingMetadata &pMetadata) {
+                                            PackingMetadata &metadata) {
 
   int64_t packedRank = packOp.getDestType().getRank();
   ArrayRef<int64_t> innerDimPos = packOp.getInnerDimsPos();
   ArrayRef<int64_t> outerPerm = packOp.getOuterDimsPerm();
   SmallVector<int64_t> packInvDestPerm =
-      computePackUnPackPerm(packedRank, innerDimPos, outerPerm, pMetadata);
+      computePackUnPackPerm(packedRank, innerDimPos, outerPerm, metadata);
   return packInvDestPerm;
 }
 
-SmallVector<int64_t> getUnPackInverseSrcPerm(UnPackOp unpackOp) {
-  PackingMetadata metadata;
-  return getUnPackInverseSrcPerm(unpackOp, metadata);
-}
-
 SmallVector<int64_t> getUnPackInverseSrcPerm(UnPackOp unpackOp,
                                              PackingMetadata &metadata) {
   int64_t packedRank = unpackOp.getSourceType().getRank();

mlir/test/Dialect/Linalg/vectorization/linalg-ops.mlir

@@ -1427,12 +1427,10 @@ module attributes {transform.with_named_sequence} {
 func.func @pack_with_dynamic_dims(
     %src: tensor<?x?xf32>, 
     %dest: tensor<?x?x16x2xf32>) -> tensor<?x?x16x2xf32> {
-
   %pack = linalg.pack %src 
     inner_dims_pos = [1, 0]
     inner_tiles = [16, 2]
     into %dest : tensor<?x?xf32> -> tensor<?x?x16x2xf32>
-
   return %pack : tensor<?x?x16x2xf32>
 }
 
@@ -1492,14 +1490,11 @@ module attributes {transform.with_named_sequence} {
 func.func @pack_with_dynamic_dims_and_dynamic_inner_tile(
     %src: tensor<?x?xf32>,
     %dest: tensor<?x?x?x2xf32>) -> tensor<?x?x?x2xf32> {
-
   %c16 = arith.constant 16 : index
-
   %pack = linalg.pack %src
     inner_dims_pos = [1, 0]
     inner_tiles = [%c16, 2]
     into %dest : tensor<?x?xf32> -> tensor<?x?x?x2xf32>
-
   return %pack : tensor<?x?x?x2xf32>
 }