[Mlir-commits] [mlir] [mlir][vector] Teach `TransferOptimization` to forward masked stores (PR #87794)
Andrzej WarzyĆski
llvmlistbot at llvm.org
Wed May 15 13:18:02 PDT 2024
================
@@ -170,12 +170,43 @@ AffineMap mlir::vector::getTransferMinorIdentityMap(ShapedType shapedType,
shapedType.getContext());
}
+/// Returns true if the value written by `defWrite` could be the same as the
+/// value read by `read`. Note: True is 'could be' not 'definitely' (as this
+/// simply looks at the masks and the value written). For a definite answer use
+/// `checkSameValueRAW()` -- which calls this function.
----------------
banach-space wrote:
Still, a `bool` method will always check a very specific condition which either holds or it does not. "could" in a function name suggests that this method is unable to return a clear answer.
I think that the problem comes from the fact that the name is tied to where it's used (`checkSameValueRAW`).
> This is a private/static method for implementing part of checkSameValueRAW(). The comment is to make clear that is function does not check enough to give a useful answer alone (so you likely don't want to call it -- you probably want checkSameValueRAW().
If this method "lives" outside of `checkSameValueRAW` then it should be able to document itself (without requiring one to scan `checkSameValueRAW`). Could you use a less ambiguous name? Here are some options:
* `haveIdenticalMasksAndMatchingPadAndSplatValues`
* `areMasksAndPadAndSplatValsConsistent`
I think that that would captures what's being checked? Alternatively, make this a lambda internal to ``checkSameValueRAW` (sounds like it's not meant to be used outside of `checkSameValueRAW` anyway).
https://github.com/llvm/llvm-project/pull/87794
More information about the Mlir-commits
mailing list