This is an archive of the discontinued LLVM Phabricator instance.

[mlir][Linalg] NFC - Refactor LinalgStructuredOps towards "named" Linalg ops
ClosedPublic

Authored by nicolasvasilache on Feb 25 2020, 3:03 PM.

Details

Reviewers
rriddle
Summary

This revision performs some basic refactoring towards more easily defining Linalg "named" ops. Such named ops form the backbone of operations that are ubiquitous in the ML application domain.

Diff Detail

Event Timeline

rriddle accepted this revision.Feb 25 2020, 3:13 PM
rriddle added inline comments.
mlir/include/mlir/Dialect/Linalg/IR/LinalgOps.h
62

nit: Please move forward declarations to the top.

mlir/include/mlir/Dialect/Linalg/IR/LinalgStructuredOps.td
361
mlir/include/mlir/Dialect/Linalg/IR/LinalgStructuredOpsInterface.td
23

Same here.

mlir/lib/Dialect/Linalg/IR/LinalgOps.cpp
903–906

Was there a problem with using llvm::concat?

This revision is now accepted and ready to land.Feb 25 2020, 3:13 PM
rriddle added inline comments.Feb 25 2020, 3:14 PM
mlir/include/mlir/Dialect/Linalg/IR/LinalgStructuredOpsInterface.td
14

LINALG_IR_STRUCTURED_OPS_INTERFACE

?

nicolasvasilache marked 6 inline comments as done.Feb 25 2020, 3:32 PM
nicolasvasilache added inline comments.
mlir/include/mlir/Dialect/Linalg/IR/LinalgStructuredOpsInterface.td
14

thanks!

mlir/lib/Dialect/Linalg/IR/LinalgOps.cpp
903–906

This

SmallVector<AffineExpr, 4> mlir::linalg::concat(ArrayRef<AffineExpr> a,
                                                ArrayRef<AffineExpr> b) {
  auto rangeA = llvm::make_range(a.begin(), a.end());
  auto rangeB = llvm::make_range(b.begin(), b.end());
  auto concatRanges = llvm::concat<AffineExpr>(rangeA, rangeB);
  return llvm::to_vector<4>(concatRanges);
}

fails with

ERROR: third_party/llvm/llvm-project/llvm/include/llvm/ADT/STLExtras.h:855:12 cannot initialize return object of type 'mlir::AffineExpr *' with an rvalue of type 'const mlir::AffineExpr *'
    return &*Begin;
           ^~~~~~~
third_party/llvm/llvm-project/llvm/include/llvm/ADT/STLExtras.h:865:27 in instantiation of function template specialization 'llvm::concat_iterator<mlir::AffineExpr, const mlir::AffineExpr *, const mlir::AffineExpr *>::getHelper<1>' requested here
        &concat_iterator::getHelper<Ns>...};
                          ^
third_party/llvm/llvm-project/llvm/include/llvm/ADT/STLExtras.h:892:12 in instantiation of function template specialization 'llvm::concat_iterator<mlir::AffineExpr, const mlir::AffineExpr *, const mlir::AffineExpr *>::get<0, 1>' requested here
    return get(std::index_sequence_for<IterTs...>());
           ^
third_party/crosstool/v18/stable/toolchain/include/c++/v1/memory:3162:75 in instantiation of member function 'llvm::concat_iterator<mlir::AffineExpr, const mlir::AffineExpr *, const mlir::AffineExpr *>::operator*' requested here
            ::new (static_cast<void*>(_VSTD::addressof(*__r))) value_type(*__f);
                                                                          ^
third_party/llvm/llvm-project/llvm/include/llvm/ADT/SmallVector.h:279:10 in instantiation of function template specialization 'std::__u::uninitialized_copy<llvm::concat_iterator<mlir::AffineExpr, const mlir::AffineExpr *, const mlir::AffineExpr *>, mlir::AffineExpr *>' requested here
    std::uninitialized_copy(I, E, Dest);
         ^
third_party/llvm/llvm-project/llvm/include/llvm/ADT/SmallVector.h:392:11 in instantiation of function template specialization 'llvm::SmallVectorTemplateBase<mlir::AffineExpr, true>::uninitialized_copy<llvm::concat_iterator<mlir::AffineExpr, const mlir::AffineExpr *, const mlir::AffineExpr *>, mlir::AffineExpr *>' requested here
    this->uninitialized_copy(in_start, in_end, this->end());
          ^
third_party/llvm/llvm-project/llvm/include/llvm/ADT/SmallVector.h:856:11 in instantiation of function template specialization 'llvm::SmallVectorImpl<mlir::AffineExpr>::append<llvm::concat_iterator<mlir::AffineExpr, const mlir::AffineExpr *, const mlir::AffineExpr *>, void>' requested here
    this->append(S, E);
          ^
third_party/llvm/llvm-project/llvm/include/llvm/ADT/SmallVector.h:918:10 in instantiation of function template specialization 'llvm::SmallVector<mlir::AffineExpr, 4>::SmallVector<llvm::concat_iterator<mlir::AffineExpr, const mlir::AffineExpr *, const mlir::AffineExpr *>, void>' requested here
  return {std::begin(Range), std::end(Range)};
         ^
third_party/llvm/llvm-project/mlir/lib/Dialect/Linalg/IR/LinalgOps.cpp:919:16 in instantiation of function template specialization 'llvm::to_vector<4, llvm::detail::concat_range<mlir::AffineExpr, llvm::iterator_range<const mlir::AffineExpr *> &, llvm::iterator_range<const mlir::AffineExpr *> &> &>' requested here
  return llvm::to_vector<4>(concatRanges);

What am I missing?

rriddle added inline comments.Feb 25 2020, 3:35 PM
mlir/lib/Dialect/Linalg/IR/LinalgOps.cpp
903–906

I think you would need to do llvm::concat<const AffineExpr>(...), given that ArrayRef holds values of const T.

nicolasvasilache marked 4 inline comments as done.Feb 25 2020, 3:52 PM
nicolasvasilache added inline comments.
mlir/lib/Dialect/Linalg/IR/LinalgOps.cpp
903–906

well .. of course, thanks!

nicolasvasilache marked an inline comment as done.

Address review comments.

Address review.

Harbormaster completed remote builds in B47294: Diff 246698.

Land as fcfd4fb686b83a5c8c7ba2c3bd4992ff24e83870 but a glitch in my local config dropped the phab link..