-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[mlir][EmitC] Add pass that combines all available emitc conversions #117549
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@llvm/pr-subscribers-mlir-func @llvm/pr-subscribers-mlir Author: Simon Camphausen (simon-camp) ChangesFull diff: https://github.com/llvm/llvm-project/pull/117549.diff 8 Files Affected:
diff --git a/mlir/include/mlir/Conversion/ConvertToEmitC/ConvertToEmitCPass.h b/mlir/include/mlir/Conversion/ConvertToEmitC/ConvertToEmitCPass.h
new file mode 100644
index 00000000000000..e6244276e6e680
--- /dev/null
+++ b/mlir/include/mlir/Conversion/ConvertToEmitC/ConvertToEmitCPass.h
@@ -0,0 +1,22 @@
+//===- ConvertToEmitCPass.h - Conversion to EmitC pass ----*- C++ -*-=========//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef MLIR_CONVERSION_CONVERTTOEMITC_CONVERTTOEMITCPASS_H
+#define MLIR_CONVERSION_CONVERTTOEMITC_CONVERTTOEMITCPASS_H
+
+#include <memory>
+
+namespace mlir {
+class Pass;
+
+#define GEN_PASS_DECL_CONVERTTOEMITCPASS
+#include "mlir/Conversion/Passes.h.inc"
+
+} // namespace mlir
+
+#endif // MLIR_CONVERSION_CONVERTTOEMITC_CONVERTTOEMITCPASS_H
diff --git a/mlir/include/mlir/Conversion/Passes.h b/mlir/include/mlir/Conversion/Passes.h
index 2ab32836c80b1c..c9698a9985aca4 100644
--- a/mlir/include/mlir/Conversion/Passes.h
+++ b/mlir/include/mlir/Conversion/Passes.h
@@ -29,6 +29,7 @@
#include "mlir/Conversion/ControlFlowToSCF/ControlFlowToSCF.h"
#include "mlir/Conversion/ControlFlowToSPIRV/ControlFlowToSPIRV.h"
#include "mlir/Conversion/ControlFlowToSPIRV/ControlFlowToSPIRVPass.h"
+#include "mlir/Conversion/ConvertToEmitC/ConvertToEmitCPass.h"
#include "mlir/Conversion/ConvertToLLVM/ToLLVMPass.h"
#include "mlir/Conversion/ConvertToSPIRV/ConvertToSPIRVPass.h"
#include "mlir/Conversion/FuncToEmitC/FuncToEmitCPass.h"
diff --git a/mlir/include/mlir/Conversion/Passes.td b/mlir/include/mlir/Conversion/Passes.td
index 4d272ba219c6f1..167b78a683501a 100644
--- a/mlir/include/mlir/Conversion/Passes.td
+++ b/mlir/include/mlir/Conversion/Passes.td
@@ -12,6 +12,20 @@
include "mlir/Pass/PassBase.td"
+//===----------------------------------------------------------------------===//
+// ToEmitC
+//===----------------------------------------------------------------------===//
+
+def ConvertToEmitCPass : Pass<"convert-to-emitc"> {
+ let summary = "Convert to EmitC dialect";
+ let description = [{
+ This is a generic pass to convert to the EmitC dialect.
+ }];
+ let dependentDialects = [
+ "emitc::EmitCDialect",
+ ];
+}
+
//===----------------------------------------------------------------------===//
// ToLLVM
//===----------------------------------------------------------------------===//
diff --git a/mlir/lib/Conversion/CMakeLists.txt b/mlir/lib/Conversion/CMakeLists.txt
index 6651d87162257f..bfee78a9624cb6 100644
--- a/mlir/lib/Conversion/CMakeLists.txt
+++ b/mlir/lib/Conversion/CMakeLists.txt
@@ -18,6 +18,7 @@ add_subdirectory(ComplexToStandard)
add_subdirectory(ControlFlowToLLVM)
add_subdirectory(ControlFlowToSCF)
add_subdirectory(ControlFlowToSPIRV)
+add_subdirectory(ConvertToEmitC)
add_subdirectory(ConvertToLLVM)
add_subdirectory(ConvertToSPIRV)
add_subdirectory(FuncToEmitC)
diff --git a/mlir/lib/Conversion/ConvertToEmitC/CMakeLists.txt b/mlir/lib/Conversion/ConvertToEmitC/CMakeLists.txt
new file mode 100644
index 00000000000000..3846f5a279a8a2
--- /dev/null
+++ b/mlir/lib/Conversion/ConvertToEmitC/CMakeLists.txt
@@ -0,0 +1,25 @@
+set(LLVM_OPTIONAL_SOURCES
+ ConvertToEmitCPass.cpp
+)
+
+add_mlir_conversion_library(MLIRConvertToEmitCPass
+ ConvertToEmitCPass.cpp
+
+ ADDITIONAL_HEADER_DIRS
+ ${MLIR_MAIN_INCLUDE_DIR}/mlir/Conversion/ConvertToEmitC
+
+ DEPENDS
+ MLIRConversionPassIncGen
+
+ LINK_LIBS PUBLIC
+ MLIRArithToEmitC
+ MLIRFuncToEmitC
+ MLIRIR
+ MLIRMemRefToEmitC
+ MLIRPass
+ MLIRRewrite
+ MLIRSCFToEmitC
+ MLIRSupport
+ MLIRTransforms
+ MLIRTransformUtils
+ )
diff --git a/mlir/lib/Conversion/ConvertToEmitC/ConvertToEmitCPass.cpp b/mlir/lib/Conversion/ConvertToEmitC/ConvertToEmitCPass.cpp
new file mode 100644
index 00000000000000..aeadb63e5faa2d
--- /dev/null
+++ b/mlir/lib/Conversion/ConvertToEmitC/ConvertToEmitCPass.cpp
@@ -0,0 +1,77 @@
+//===- ConvertToEmitCPass.cpp - MLIR EmitC Conversion ---------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "mlir/Conversion/ConvertToEmitC/ConvertToEmitCPass.h"
+#include "mlir/Conversion/ArithToEmitC/ArithToEmitC.h"
+#include "mlir/Conversion/FuncToEmitC/FuncToEmitC.h"
+#include "mlir/Conversion/MemRefToEmitC/MemRefToEmitC.h"
+#include "mlir/Conversion/SCFToEmitC/SCFToEmitC.h"
+#include "mlir/Dialect/Arith/IR/Arith.h"
+#include "mlir/Dialect/EmitC/IR/EmitC.h"
+#include "mlir/Dialect/Func/IR/FuncOps.h"
+#include "mlir/Dialect/MemRef/IR/MemRef.h"
+#include "mlir/Dialect/SCF/IR/SCF.h"
+#include "mlir/IR/PatternMatch.h"
+#include "mlir/Pass/Pass.h"
+#include "mlir/Rewrite/FrozenRewritePatternSet.h"
+#include "mlir/Transforms/DialectConversion.h"
+#include "mlir/Transforms/GreedyPatternRewriteDriver.h"
+
+#define DEBUG_TYPE "convert-to-emitc"
+
+namespace mlir {
+#define GEN_PASS_DEF_CONVERTTOEMITCPASS
+#include "mlir/Conversion/Passes.h.inc"
+} // namespace mlir
+
+using namespace mlir;
+
+namespace {
+
+/// Populate patterns for each dialect.
+void populateConvertToEmitCPatterns(TypeConverter &typeConverter,
+ RewritePatternSet &patterns) {
+ populateArithToEmitCPatterns(typeConverter, patterns);
+ populateFuncToEmitCPatterns(patterns);
+ populateMemRefToEmitCTypeConversion(typeConverter);
+ populateMemRefToEmitCConversionPatterns(patterns, typeConverter);
+ populateSCFToEmitCConversionPatterns(patterns);
+ populateFunctionOpInterfaceTypeConversionPattern<emitc::FuncOp>(
+ patterns, typeConverter);
+}
+
+/// A pass to perform the SPIR-V conversion.
+struct ConvertToEmitCPass final
+ : impl::ConvertToEmitCPassBase<ConvertToEmitCPass> {
+ using ConvertToEmitCPassBase::ConvertToEmitCPassBase;
+
+ void runOnOperation() override {
+ Operation *op = getOperation();
+ MLIRContext *context = &getContext();
+
+ RewritePatternSet patterns(context);
+ TypeConverter typeConverter;
+ typeConverter.addConversion([](Type type) { return type; });
+
+ ConversionTarget target(*context);
+ target.addIllegalDialect<arith::ArithDialect, func::FuncDialect,
+ memref::MemRefDialect, scf::SCFDialect>();
+ target.addLegalDialect<emitc::EmitCDialect>();
+ target.addDynamicallyLegalOp<emitc::FuncOp>(
+ [&typeConverter](emitc::FuncOp op) {
+ return typeConverter.isSignatureLegal(op.getFunctionType());
+ });
+
+ populateConvertToEmitCPatterns(typeConverter, patterns);
+ if (failed(applyPartialConversion(op, target, std::move(patterns))))
+ return signalPassFailure();
+ return;
+ }
+};
+
+} // namespace
diff --git a/mlir/test/Conversion/ConvertToEmitC/func-signature.mlir b/mlir/test/Conversion/ConvertToEmitC/func-signature.mlir
new file mode 100644
index 00000000000000..40b898919a7c22
--- /dev/null
+++ b/mlir/test/Conversion/ConvertToEmitC/func-signature.mlir
@@ -0,0 +1,19 @@
+// RUN: mlir-opt -convert-to-emitc %s | FileCheck %s
+
+// CHECK-LABEL emitc.func @int(%[[ARG:.*]]: i32)
+func.func @int(%arg0: i32) {
+ // CHECK: return
+ return
+}
+
+// CHECK-LABEL emitc.func @index(%[[ARG:.*]]: !emitc.size_t)
+func.func @index(%arg0: index) {
+ // CHECK: return
+ return
+}
+
+// CHECK-LABEL emitc.func @memref(%[[ARG:.*]]: !emitc.array<1xf32>)
+func.func @memref(%arg0: memref<1xf32>) {
+ // CHECK: return
+ return
+}
diff --git a/mlir/test/Conversion/ConvertToEmitC/tosa.mlir b/mlir/test/Conversion/ConvertToEmitC/tosa.mlir
new file mode 100644
index 00000000000000..4a7466bd04ba8b
--- /dev/null
+++ b/mlir/test/Conversion/ConvertToEmitC/tosa.mlir
@@ -0,0 +1,46 @@
+// DEFINE: %{pipeline} = "builtin.module(\
+// DEFINE: func.func(\
+// DEFINE: tosa-to-linalg\
+// DEFINE: ),\
+// DEFINE: one-shot-bufferize{\
+// DEFINE: bufferize-function-boundaries\
+// DEFINE: function-boundary-type-conversion=identity-layout-map\
+// DEFINE: buffer-alignment=0\
+// DEFINE: },\
+// DEFINE: buffer-results-to-out-params{\
+// DEFINE: hoist-static-allocs=true\
+// DEFINE: },\
+// DEFINE: func.func(\
+// DEFINE: convert-linalg-to-loops\
+// DEFINE: ),\
+// DEFINE: canonicalize,\
+// DEFINE: convert-to-emitc\
+// DEFINE: )"
+
+// RUN: mlir-opt --pass-pipeline=%{pipeline} %s | FileCheck %s
+
+// -----
+
+// CHECK: emitc.func @main(%[[ARG0:.*]]: !emitc.array<2xf32>, %[[ARG1:.*]]: !emitc.array<2xf32>, %[[RES:.*]]: !emitc.array<2xf32>) {
+// CHECK-DAG: %[[C0:.*]] = "emitc.constant"() <{value = 0 : index}> : () -> !emitc.size_t
+// CHECK-DAG: %[[C1:.*]] = "emitc.constant"() <{value = 1 : index}> : () -> !emitc.size_t
+// CHECK-DAG: %[[C2:.*]] = "emitc.constant"() <{value = 2 : index}> : () -> !emitc.size_t
+// CHECK-DAG: %[[I0:.*]] = builtin.unrealized_conversion_cast %[[C0]] : !emitc.size_t to index
+// CHECK-DAG: %[[I1:.*]] = builtin.unrealized_conversion_cast %[[C1]] : !emitc.size_t to index
+// CHECK-DAG: %[[I2:.*]] = builtin.unrealized_conversion_cast %[[C2]] : !emitc.size_t to index
+// CHECK-NEXT: for %[[INDEX:.*]] = %[[I0]] to %[[I2]] step %[[I1]] {
+// CHECK-NEXT: %[[INDEX_S:.*]] = builtin.unrealized_conversion_cast %[[INDEX]] : index to !emitc.size_t
+// CHECK-NEXT: %[[V0_LVALUE:.*]] = emitc.subscript %[[ARG0]][%[[INDEX_S]]] : (!emitc.array<2xf32>, !emitc.size_t) -> !emitc.lvalue<f32>
+// CHECK-NEXT: %[[V0:.*]] = emitc.load %[[V0_LVALUE]] : <f32>
+// CHECK-NEXT: %[[V1_LVALUE:.*]] = emitc.subscript %[[ARG1]][%[[INDEX_S]]] : (!emitc.array<2xf32>, !emitc.size_t) -> !emitc.lvalue<f32>
+// CHECK-NEXT: %[[V1:.*]] = emitc.load %[[V1_LVALUE]] : <f32>
+// CHECK-NEXT: %[[VADD:.*]] = emitc.add %[[V0]], %[[V1]] : (f32, f32) -> f32
+// CHECK-NEXT: %[[RES_LVALUE:.*]] = emitc.subscript %[[RES]][%[[INDEX_S]]] : (!emitc.array<2xf32>, !emitc.size_t) -> !emitc.lvalue<f32>
+// CHECK-NEXT: emitc.assign %[[VADD]] : f32 to %[[RES_LVALUE]] : <f32>
+// CHECK-NEXT: }
+// CHECK-NEXT: return
+// CHECK-NEXT: }
+func.func @main(%arg0: tensor<2xf32>, %arg1: tensor<2xf32>) -> tensor<2xf32> {
+ %0 = tosa.add %arg0, %arg1 : (tensor<2xf32>, tensor<2xf32>) -> tensor<2xf32>
+ return %0 : tensor<2xf32>
+}
|
func.func @memref(%arg0: memref<1xf32>) { | ||
// CHECK: return | ||
return | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we also add one test each for the arith
and scf
dialects?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Some NITs.
✅ With the latest revision this PR passed the C/C++ code formatter. |
@@ -16,7 +16,7 @@ include "mlir/Pass/PassBase.td" | |||
// ToEmitC | |||
//===----------------------------------------------------------------------===// | |||
|
|||
def ConvertToEmitCPass : Pass<"convert-to-emitc"> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm confused because this PR says "Add pass ...." but here you're just using an existing pass?
Can you update the PR title and provide a description explaining the changes?
@simon-camp , now that #118940 is landed, are there any blockers to rebasing this work and following up w/ @joker-eph's suggestion? |
I think this is a private contribution now and I am not sure about resources on Simon's side. |
Then it would be good to get clarity on if @simon-camp intends to continue this work, or if we should find someone who can take over, and if there are any remaining patches required before that work could continue. |
Hi, I will continue working in this next week. |
@simon-camp Thanks for the clarification. Let me know if you want/need any help. |
@simon-camp any progress to report? |
I have something partly working modelled Like the LLVM interface. I will push it tomorrow. |
1b8aacb
to
b5bdbaf
Compare
@simon-camp it seems like there are some issues with this patch, based on the test failures. I'm afraid I'm not familiar enough here to offer much advice, but it seems like some of the tests, like
I've also looked at the EmitC output for the TOSA model. While the TOSA to EmitC test seems to be able to generate C code after being passed through |
I assume with TOSA to EmitC you refer to what we provided with iml130/mlir-emitc (Public archive |
This seems to be a layering issue. The memref->emitc type conversion is only available when the memref dialect is loaded. And that dialect is not loaded for the test. Nonetheless there are more failing tests I need to fix. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
@marbre should probably also have another look here? |
Sure, I free up some time to review latest early next week. |
@marbre, others - would you terribly mind if I merged this PR - I want to build on it and it would be easier if it were merged. From the comments so far, I'm assuming you wouldn't have strong pushbacks on the PR, and we could address additional feedback after it merged, which I'm happy to take on. WDYT? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please excuse the delayed review. I am not familiar with all the details using the interface but looks overall pretty good to me. Just minor nits but feel free to land without further review.
@@ -73,6 +74,7 @@ void SCFDialect::initialize() { | |||
#include "mlir/Dialect/SCF/IR/SCFOps.cpp.inc" | |||
>(); | |||
addInterfaces<SCFInlinerInterface>(); | |||
declarePromisedInterface<ConvertToEmitCPatternInterface, SCFDialect>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might want to move this down before declarePromisedInterface<ValueBoundsOpInterface, ForOp>();
?
Co-authored-by: Marius Brehler <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, thanks!
Thanks! I'll go ahead and push the button, and follow up with a patch for bazel and any other cleanup. Very excited to build on EmitC! |
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/207/builds/903 Here is the relevant piece of the build log for the reference
|
Follow-up from #117549 tested by: ``` cd utils/bazel bazelisk build --config=generic_clang @llvm-project//mlir:all bazelisk test --config=generic_clang @llvm-project//mlir/test:all ```
@simon-camp @mtrofin I've landed 514b853 to fix the mlir build under |
Thanks! |
Follow-up from llvm#117549 tested by: ``` cd utils/bazel bazelisk build --config=generic_clang @llvm-project//mlir:all bazelisk test --config=generic_clang @llvm-project//mlir/test:all ```
Follow-up from llvm#117549 tested by: ``` cd utils/bazel bazelisk build --config=generic_clang @llvm-project//mlir:all bazelisk test --config=generic_clang @llvm-project//mlir/test:all ```
Follow-up from llvm#117549 tested by: ``` cd utils/bazel bazelisk build --config=generic_clang @llvm-project//mlir:all bazelisk test --config=generic_clang @llvm-project//mlir/test:all ```
Follow-up from llvm#117549 tested by: ``` cd utils/bazel bazelisk build --config=generic_clang @llvm-project//mlir:all bazelisk test --config=generic_clang @llvm-project//mlir/test:all ```
Follow-up from llvm#117549 tested by: ``` cd utils/bazel bazelisk build --config=generic_clang @llvm-project//mlir:all bazelisk test --config=generic_clang @llvm-project//mlir/test:all ```
No description provided.