Page MenuHomePhabricator

[NFC][mlir] Adding some helpful EDSC intrinsics
ClosedPublic

Authored by kernhanda on Thu, Feb 6, 4:16 AM.

Diff Detail

Event Timeline

kernhanda created this revision.Thu, Feb 6, 4:16 AM
Herald added a project: Restricted Project. · View Herald TranscriptThu, Feb 6, 4:16 AM
kernhanda retitled this revision from [mlir] Adding some helpful EDSC intrinsics to [NFC][mlir] Adding some helpful EDSC intrinsics.Thu, Feb 6, 4:16 AM
ftynse added inline comments.Thu, Feb 6, 5:41 AM
mlir/include/mlir/EDSC/Intrinsics.h
124

Please make sure the name corresponds to the instruction name, e.g. ConstantOp should be just constant.

kernhanda updated this revision to Diff 242966.Thu, Feb 6, 11:37 AM

constant_data -> constant

kernhanda marked an inline comment as done.Thu, Feb 6, 11:39 AM
ftynse accepted this revision.Thu, Feb 6, 2:27 PM
This revision is now accepted and ready to land.Thu, Feb 6, 2:27 PM

Someone will have to commit this on my behalf.

nicolasvasilache requested changes to this revision.Fri, Feb 7, 11:15 AM
nicolasvasilache added inline comments.
mlir/include/mlir/EDSC/Intrinsics.h
124

this breaks the build, please fix the other use case:

llvm-project/mlir/lib/Conversion/LinalgToLLVM/LinalgToLLVM.cpp: In member function ‘virtual mlir::PatternMatchResult {anonymous}::SliceOpConversion::matchAndRewrite(mlir::Operation*, llvm::ArrayRef<mlir::Value>, mlir::ConversionPatternRewriter&) const’:
llvm-project/mlir/lib/Conversion/LinalgToLLVM/LinalgToLLVM.cpp:268:9: error: reference to ‘constant’ is ambiguous
  268 |         constant(int64Ty, rewriter.getIntegerAttr(rewriter.getIndexType(), 0));
      |         ^~~~~~~~
In file included from llvm-project/mlir/lib/Conversion/LinalgToLLVM/LinalgToLLVM.cpp:20:
llvm-project/mlir/include/mlir/EDSC/Intrinsics.h:203:7: note: candidates are: ‘using constant = struct mlir::edsc::intrinsics::ValueBuilder<mlir::ConstantOp>’
  203 | using constant = ValueBuilder<ConstantOp>;
      |       ^~~~~~~~
llvm-project/mlir/lib/Conversion/LinalgToLLVM/LinalgToLLVM.cpp:54:7: note:                 ‘using constant = struct mlir::edsc::intrinsics::ValueBuilder<mlir::LLVM::ConstantOp>’
   54 | using constant = ValueBuilder<mlir::LLVM::ConstantOp>;
      |       ^~~~~~~~
This revision now requires changes to proceed.Fri, Feb 7, 11:15 AM
kernhanda updated this revision to Diff 243349.Fri, Feb 7, 11:25 PM

fixing build fix

kernhanda updated this revision to Diff 243350.Fri, Feb 7, 11:27 PM
kernhanda marked an inline comment as done.

ran clang-format

kernhanda marked an inline comment as done.Fri, Feb 7, 11:28 PM
kernhanda added inline comments.
mlir/include/mlir/EDSC/Intrinsics.h
124

Thanks for catching this.

Ping. :) Someone will also have to commit this on my behalf.

My apologies for the delay..
Also, please note that https://github.com/llvm/llvm-project/commit/75394e1301dd1d4739a24d440e9996c52e5b5570#diff-6635fa1a5b0b10edf6e511585c7c986d refactored things significantly on the EDSC front so this would need a rebase.

This revision was not accepted when it landed; it landed in state Needs Review.Thu, Feb 13, 12:23 AM
This revision was automatically updated to reflect the committed changes.