[PATCH] D79489: [mlir][Linalg] NFC - Refactor and simplify Promotion

[Mahesh Ravishankar via Phabricator via llvm-commits]

mravishankar added inline comments.


================
Comment at: mlir/include/mlir/Dialect/Linalg/Transforms/Transforms.h:267
+/// See `promoteSubViews` for more details.
 struct LinalgBasePromotionPattern : public RewritePattern {
   LinalgBasePromotionPattern(StringRef opName, MLIRContext *context,
----------------
mravishankar wrote:
> This seems to diverge from the way patterns are exposed in other places. The convention is AFAIK to use "populate.....Pattern" which takes a OwningRewritePatternList as an argument (by reference) and inserts the pattern. With that you dont need to have the pattern itself exposed just an API to insert the pattern. You could then add as many patterns as you want into that.
Actually I see now how this is supposed to work. Disregard.


================
Comment at: mlir/lib/Dialect/Linalg/Transforms/Promotion.cpp:208
     // boundary.
     linalg_fill(info->second.fullLocalView, fillVal);
   }
----------------
I think these fill needs to have the marker added to it too. If not they can be intercepted by the pattern rewriter which would try to apply transformations on them (AFAIK the marker is supposed to prevent that).
Same is true for the linalg_copy below.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79489