[PATCH] D72112: [mlir][Linalg] NFC - Post-commit cleanups

River Riddle via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Jan 5 21:19:10 PST 2020


rriddle added inline comments.


================
Comment at: mlir/include/mlir/Support/Functional.h:50
+template <typename Fn, typename IterType>
+auto map_filter(Fn fun, IterType begin, IterType end)
+    -> SmallVector<typename std::result_of<Fn(decltype(*begin))>::type, 8> {
----------------
rriddle wrote:
> nicolasvasilache wrote:
> > rriddle wrote:
> > > This seems hardcoded for a very specific type of iterator element, i.e. one where the result is nullable.
> > I can rename to `map_if_non_null`.
> > I was reluctant on forcing a second lambda and turn into a `map_if` but I could do that too and impl `map_if_non_null` in terms of that.
> > Thoughts?
> I'm not convinced that these methods are worth it. This looks like a case that is already covered by a combination of map+filter ranges.
FWIW, this also applies to other things within this file; e.g. I don't really see the benefit of 'map' when you can use llvm::map_range instead. At that point, you can opt into vector materialization if you actually need it. This file seems to be reimplementing many of the iterator ranges, albeit using SmallVector instead of using iterator ranges.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72112





More information about the llvm-commits mailing list