[PATCH] D133166: [mlir][MemRef] Canonicalize extract_strided_metadata(subview)

[Nicolas Vasilache via Phabricator via llvm-commits]

nicolasvasilache accepted this revision.
nicolasvasilache added inline comments.


================
Comment at: mlir/lib/Dialect/MemRef/Transforms/SimplifyExtractStridedMetadata.cpp:112
+      strides.push_back(makeComposedFoldedAffineApply(
+          rewriter, origLoc, s0 * s1, {subStrides[i], getOrigStrideAtIdx(i)}));
+
----------------
qcolombet wrote:
> qcolombet wrote:
> > nicolasvasilache wrote:
> > > This feels like too many levels of indirection for no good reason.
> > > How about something resembling:
> > > ```
> > > for () {
> > >   OpFoldResult origStride = (ShapedType::isDynamicStrideOrOffset(sourceStrides[i])) ? 
> > >     b.getIndexAttr(sourceStrides[i])  : origStrides[i];
> > >    strides.push_back(makeComposedFoldedAffineApply(
> > >        rewriter, origLoc, s0 * s1, {subStrides[i], origStride}));
> > > }
> > > ```
> > Sounds like I should merge the loop for the strides and the loop of the offset then.
> > Otherwise I will repeat in both loops:
> > ```
> >   OpFoldResult origStride = (ShapedType::isDynamicStrideOrOffset(sourceStrides[i])) ? 
> >     b.getIndexAttr(sourceStrides[i])  : origStrides[i];
> > ```
> @nicolasvasilache I'm going to leave the PR open until tomorrow to give you a chance to see the merged loops.
> (In other words, what I did to avoid the duplication of the ternary op for the original stride after getting rid of the lambda.)
WFM, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133166