-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[mlir][emitc] Add op modelling C expressions #71631
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-emitc @llvm/pr-subscribers-mlir Author: Gil Rapaport (aniragil) ChangesAdd an emitc.expression operation that models C expressions, and provide Patch is 69.50 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/71631.diff 24 Files Affected:
diff --git a/mlir/include/mlir/Dialect/EmitC/CMakeLists.txt b/mlir/include/mlir/Dialect/EmitC/CMakeLists.txt
index f33061b2d87cffc..cb1e9d01821a2cf 100644
--- a/mlir/include/mlir/Dialect/EmitC/CMakeLists.txt
+++ b/mlir/include/mlir/Dialect/EmitC/CMakeLists.txt
@@ -1 +1,3 @@
add_subdirectory(IR)
+add_subdirectory(Transforms)
+add_subdirectory(TransformOps)
diff --git a/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td b/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td
index 2edeb6f8a9cf01e..4d522565c32826d 100644
--- a/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td
+++ b/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td
@@ -19,6 +19,7 @@ include "mlir/Dialect/EmitC/IR/EmitCTypes.td"
include "mlir/Interfaces/CastInterfaces.td"
include "mlir/Interfaces/ControlFlowInterfaces.td"
include "mlir/Interfaces/SideEffectInterfaces.td"
+include "mlir/IR/RegionKindInterface.td"
//===----------------------------------------------------------------------===//
// EmitC op definitions
@@ -246,6 +247,85 @@ def EmitC_DivOp : EmitC_BinaryOp<"div", []> {
let results = (outs FloatIntegerIndexOrOpaqueType);
}
+def EmitC_ExpressionOp : EmitC_Op<"expression",
+ [HasOnlyGraphRegion, SingleBlockImplicitTerminator<"emitc::YieldOp">,
+ NoRegionArguments]> {
+ let summary = "Expression operation";
+ let description = [{
+ The `expression` operation returns a single SSA value which is yielded by
+ its single-basic-block region. The operation doesn't take any arguments.
+
+ As the operation is to be emitted as a C expression, the operations within
+ its body must form a single Def-Use tree of emitc ops whose result is
+ yielded by a terminating `yield`.
+
+ Example:
+
+ ```mlir
+ %r = emitc.expression : () -> i32 {
+ %0 = emitc.add %a, %b : (i32, i32) -> i32
+ %1 = emitc.call "foo"(%0) : () -> i32
+ %2 = emitc.add %c, %d : (i32, i32) -> i32
+ %3 = emitc.mul %1, %2 : (i32, i32) -> i32
+ yield %3
+ }
+ ```
+
+ May be emitted as
+
+ ```c++
+ int32_t v7 = foo(v1 + v2) * (v3 + v4);
+ ```
+
+ The operations allowed within expression body are emitc.add, emitc.apply,
+ emitc.call, emitc.cast, emitc.cmp, emitc.div, emitc.mul, emitc.rem and
+ emitc.sub.
+
+ When specified, the optional do_not_inline indicates that the expression is
+ to be emitted as seen above, i.e. as the rhs of an EmitC SSA value
+ definition. Otherwise, the expression may be emitted inline, i.e. directly
+ at its use.
+ }];
+
+ let arguments = (ins UnitAttr:$do_not_inline);
+ let results = (outs AnyType);
+ let regions = (region SizedRegion<1>:$region);
+
+ let hasVerifier = 1;
+ let hasCustomAssemblyFormat = 1;
+
+ let extraClassDeclaration = [{
+ static bool isCExpression(Operation &op) {
+ return isa<emitc::AddOp, emitc::ApplyOp, emitc::CallOp, emitc::CastOp,
+ emitc::CmpOp, emitc::DivOp, emitc::MulOp, emitc::RemOp,
+ emitc::SubOp>(op);
+ }
+ static bool hasSideEffects(Operation &op) {
+ assert(isCExpression(op) && "Expected a C operator");
+ // Conservatively assume calls to read and write memory.
+ if (isa<emitc::CallOp>(op))
+ return true;
+ // De-referencing reads modifiable memory.
+ auto applyOp = dyn_cast<emitc::ApplyOp>(op);
+ if (applyOp && applyOp.getApplicableOperator() == "*")
+ return true;
+ // Any operator using variables has a side effect of reading memory mutable by
+ // emitc::assign ops.
+ for (Value operand : op.getOperands()) {
+ Operation *def = operand.getDefiningOp();
+ if (def && isa<emitc::VariableOp>(def))
+ return true;
+ }
+ return false;
+ }
+ bool hasSideEffects() {
+ return llvm::any_of(getRegion().front().without_terminator(),
+ [](Operation &op) { return hasSideEffects(op); });
+ }
+ Operation *getRootOp();
+ }];
+}
+
def EmitC_ForOp : EmitC_Op<"for",
[AllTypesMatch<["lowerBound", "upperBound", "step"]>,
SingleBlockImplicitTerminator<"emitc::YieldOp">,
@@ -492,18 +572,24 @@ def EmitC_AssignOp : EmitC_Op<"assign", []> {
}
def EmitC_YieldOp : EmitC_Op<"yield",
- [Pure, Terminator, ParentOneOf<["IfOp", "ForOp"]>]> {
+ [Pure, Terminator, ParentOneOf<["ExpressionOp", "IfOp", "ForOp"]>]> {
let summary = "block termination operation";
let description = [{
- "yield" terminates blocks within EmitC control-flow operations. Since
- control-flow constructs in C do not return values, this operation doesn't
- take any arguments.
+ "yield" terminates its parent EmitC op's region, optionally yielding
+ an SSA value. The semantics of how the values are yielded is defined by the
+ parent operation.
+ If "yield" has an operand, the operand must match the parent operation's
+ result. If the parent operation defines no values, then the "emitc.yield"
+ may be left out in the custom syntax and the builders will insert one
+ implicitly. Otherwise, it has to be present in the syntax to indicate which
+ value is yielded.
}];
- let arguments = (ins);
+ let arguments = (ins Optional<AnyType>:$result);
let builders = [OpBuilder<(ins), [{ /* nothing to do */ }]>];
- let assemblyFormat = [{ attr-dict }];
+ let hasVerifier = 1;
+ let assemblyFormat = [{ attr-dict ($result^ `:` type($result))? }];
}
def EmitC_IfOp : EmitC_Op<"if",
diff --git a/mlir/include/mlir/Dialect/EmitC/TransformOps/CMakeLists.txt b/mlir/include/mlir/Dialect/EmitC/TransformOps/CMakeLists.txt
new file mode 100644
index 000000000000000..364398d2dc6b4eb
--- /dev/null
+++ b/mlir/include/mlir/Dialect/EmitC/TransformOps/CMakeLists.txt
@@ -0,0 +1,6 @@
+set(LLVM_TARGET_DEFINITIONS EmitCTransformOps.td)
+mlir_tablegen(EmitCTransformOps.h.inc -gen-op-decls)
+mlir_tablegen(EmitCTransformOps.cpp.inc -gen-op-defs)
+add_public_tablegen_target(MLIREmitCTransformOpsIncGen)
+
+add_mlir_doc(EmitCTransformOps EmitCLoopTransformOps Dialects/ -gen-op-doc)
diff --git a/mlir/include/mlir/Dialect/EmitC/TransformOps/EmitCTransformOps.h b/mlir/include/mlir/Dialect/EmitC/TransformOps/EmitCTransformOps.h
new file mode 100644
index 000000000000000..5b31080c70d9fcb
--- /dev/null
+++ b/mlir/include/mlir/Dialect/EmitC/TransformOps/EmitCTransformOps.h
@@ -0,0 +1,49 @@
+//===- EmitCTransformOps.h - EmitC transformation ops ---------------*- 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_DIALECT_EMITC_TRANSFORMOPS_EmitCTRANSFORMOPS_H
+#define MLIR_DIALECT_EMITC_TRANSFORMOPS_EmitCTRANSFORMOPS_H
+
+#include "mlir/Bytecode/BytecodeOpInterface.h"
+#include "mlir/Dialect/Transform/IR/MatchInterfaces.h"
+#include "mlir/Dialect/Transform/IR/TransformDialect.h"
+#include "mlir/Dialect/Transform/IR/TransformInterfaces.h"
+#include "mlir/Dialect/Transform/IR/TransformTypes.h"
+#include "mlir/IR/OpImplementation.h"
+
+namespace mlir {
+namespace func {
+class FuncOp;
+} // namespace func
+namespace emitc {
+class ExpressionOp;
+class BinaryOp;
+} // namespace emitc
+} // namespace mlir
+
+namespace mlir {
+class DialectRegistry;
+
+namespace emitc {
+void registerTransformDialectExtension(DialectRegistry ®istry);
+} // namespace emitc
+} // namespace mlir
+
+#define GET_OP_CLASSES
+#include "mlir/Dialect/EmitC/TransformOps/EmitCTransformOps.h.inc"
+
+namespace mlir {
+class DialectRegistry;
+
+namespace scf {
+void registerTransformDialectExtension(DialectRegistry ®istry);
+} // namespace scf
+} // namespace mlir
+
+#endif // MLIR_DIALECT_EmitC_TRANSFORMOPS_EMITCTRANSFORMOPS_H
diff --git a/mlir/include/mlir/Dialect/EmitC/TransformOps/EmitCTransformOps.td b/mlir/include/mlir/Dialect/EmitC/TransformOps/EmitCTransformOps.td
new file mode 100644
index 000000000000000..1e442af2e4c1edb
--- /dev/null
+++ b/mlir/include/mlir/Dialect/EmitC/TransformOps/EmitCTransformOps.td
@@ -0,0 +1,70 @@
+//===- EmitCTransformOps.td - EmitC (loop) transformation ops --*- tablegen -*-===//
+//
+// 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 EMITC_TRANSFORM_OPS
+#define EMITC_TRANSFORM_OPS
+
+include "mlir/Dialect/Transform/IR/MatchInterfaces.td"
+include "mlir/Dialect/Transform/IR/TransformDialect.td"
+include "mlir/Dialect/Transform/IR/TransformInterfaces.td"
+include "mlir/Dialect/Transform/IR/TransformTypes.td"
+include "mlir/Interfaces/SideEffectInterfaces.td"
+include "mlir/IR/OpBase.td"
+
+def ApplyExpressionPatternsOp : Op<Transform_Dialect,
+ "apply_patterns.emitc.expressions",
+ [DeclareOpInterfaceMethods<PatternDescriptorOpInterface>]> {
+ let description = [{
+ Apply expression-related patterns.
+ }];
+
+ let arguments = (ins);
+ let assemblyFormat = [{ attr-dict }];
+}
+
+def CreateExpressionOp : Op<Transform_Dialect, "expression.create",
+ [DeclareOpInterfaceMethods<TransformOpInterface>,
+ DeclareOpInterfaceMethods<MemoryEffectsOpInterface>]> {
+ let summary = "Wrap C operators in emitc.expressions";
+ let description = [{
+ For each payload operation, constructs an `emitc.expression` wrapping that
+ operation and yielding the value it defines.
+
+ #### Return Modes
+
+ Produces a silenceable failure if the operand is not associated with emitc C
+ operator payload operations. Returns a single handle associated with the
+ generated `emitc.expression` ops.
+ }];
+ let arguments = (ins TransformHandleTypeInterface:$target);
+ let results = (outs TransformHandleTypeInterface:$transformed);
+
+ let assemblyFormat = "$target attr-dict `:` functional-type(operands, results)";
+}
+
+def MatchExpressionRootOp : Op<Transform_Dialect, "expression.match.root",
+ [MemoryEffectsOpInterface,
+ NavigationTransformOpTrait,
+ DeclareOpInterfaceMethods<TransformOpInterface>]> {
+ let description = [{
+ Match emitc ops modelling C expressions.
+ For this to be doable, the payload must have a single use, where the user is
+ another emitc::expression and the payload must be movable to just before its
+ user. Produces a silenceable failure otherwise.
+ }];
+
+ let arguments = (ins TransformHandleTypeInterface:$target);
+ let results = (outs TransformHandleTypeInterface:$results);
+
+ let assemblyFormat = [{
+ `in` $target attr-dict
+ `:` functional-type($target, results)
+ }];
+}
+
+#endif // EMITC_TRANSFORM_OPS
diff --git a/mlir/include/mlir/Dialect/EmitC/Transforms/CMakeLists.txt b/mlir/include/mlir/Dialect/EmitC/Transforms/CMakeLists.txt
new file mode 100644
index 000000000000000..0b507d75fa07a6b
--- /dev/null
+++ b/mlir/include/mlir/Dialect/EmitC/Transforms/CMakeLists.txt
@@ -0,0 +1,5 @@
+set(LLVM_TARGET_DEFINITIONS Passes.td)
+mlir_tablegen(Passes.h.inc -gen-pass-decls -name EmitC)
+add_public_tablegen_target(MLIREmitCTransformsIncGen)
+
+add_mlir_doc(Passes EmitCPasses ./ -gen-pass-doc)
diff --git a/mlir/include/mlir/Dialect/EmitC/Transforms/Passes.h b/mlir/include/mlir/Dialect/EmitC/Transforms/Passes.h
new file mode 100644
index 000000000000000..5cd27149d366ea0
--- /dev/null
+++ b/mlir/include/mlir/Dialect/EmitC/Transforms/Passes.h
@@ -0,0 +1,35 @@
+//===- Passes.h - Pass Entrypoints ------------------------------*- 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_DIALECT_EMITC_TRANSFORMS_PASSES_H_
+#define MLIR_DIALECT_EMITC_TRANSFORMS_PASSES_H_
+
+#include "mlir/Pass/Pass.h"
+
+namespace mlir {
+namespace emitc {
+
+//===----------------------------------------------------------------------===//
+// Passes
+//===----------------------------------------------------------------------===//
+
+/// Creates an instance of the C-style expressions forming pass.
+std::unique_ptr<Pass> createFormExpressionsPass();
+
+//===----------------------------------------------------------------------===//
+// Registration
+//===----------------------------------------------------------------------===//
+
+/// Generate the code for registering passes.
+#define GEN_PASS_REGISTRATION
+#include "mlir/Dialect/EmitC/Transforms/Passes.h.inc"
+
+} // namespace emitc
+} // namespace mlir
+
+#endif // MLIR_DIALECT_EMITC_TRANSFORMS_PASSES_H_
diff --git a/mlir/include/mlir/Dialect/EmitC/Transforms/Passes.td b/mlir/include/mlir/Dialect/EmitC/Transforms/Passes.td
new file mode 100644
index 000000000000000..fd083abc9571578
--- /dev/null
+++ b/mlir/include/mlir/Dialect/EmitC/Transforms/Passes.td
@@ -0,0 +1,24 @@
+//===-- Passes.td - pass definition file -------------------*- tablegen -*-===//
+//
+// 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_DIALECT_EMITC_TRANSFORMS_PASSES
+#define MLIR_DIALECT_EMITC_TRANSFORMS_PASSES
+
+include "mlir/Pass/PassBase.td"
+
+def FormExpressions : Pass<"form-expressions"> {
+ let summary = "Form C-style expressions from C-operator ops";
+ let description = [{
+ The pass wraps emitc ops modelling C operators in emitc.expression ops and
+ then folds single-use expressions into their users where possible.
+ }];
+ let constructor = "mlir::emitc::createFormExpressionsPass()";
+ let dependentDialects = ["emitc::EmitCDialect"];
+}
+
+#endif // MLIR_DIALECT_EMITC_TRANSFORMS_PASSES
diff --git a/mlir/include/mlir/Dialect/EmitC/Transforms/Transforms.h b/mlir/include/mlir/Dialect/EmitC/Transforms/Transforms.h
new file mode 100644
index 000000000000000..73981df131681c8
--- /dev/null
+++ b/mlir/include/mlir/Dialect/EmitC/Transforms/Transforms.h
@@ -0,0 +1,34 @@
+//===- Transforms.h - EmitC transformations as patterns --------*- 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_DIALECT_EMITC_TRANSFORMS_TRANSFORMS_H
+#define MLIR_DIALECT_EMITC_TRANSFORMS_TRANSFORMS_H
+
+#include "mlir/Dialect/EmitC/IR/EmitC.h"
+#include "mlir/IR/PatternMatch.h"
+
+namespace mlir {
+namespace emitc {
+
+//===----------------------------------------------------------------------===//
+// Expression transforms.
+//===----------------------------------------------------------------------===//
+
+ExpressionOp createExpression(Operation *op, OpBuilder &builder);
+
+//===----------------------------------------------------------------------===//
+// Populate functions.
+//===----------------------------------------------------------------------===//
+
+/// Populates `patterns` with expression-related patterns.
+void populateExpressionPatterns(RewritePatternSet &patterns);
+
+} // namespace emitc
+} // namespace mlir
+
+#endif // MLIR_DIALECT_EMITC_TRANSFORMS_TRANSFORMS_H
diff --git a/mlir/include/mlir/InitAllExtensions.h b/mlir/include/mlir/InitAllExtensions.h
index c04ce850fb96f41..0f494f296cd6624 100644
--- a/mlir/include/mlir/InitAllExtensions.h
+++ b/mlir/include/mlir/InitAllExtensions.h
@@ -25,6 +25,7 @@
#include "mlir/Conversion/UBToLLVM/UBToLLVM.h"
#include "mlir/Dialect/Affine/TransformOps/AffineTransformOps.h"
#include "mlir/Dialect/Bufferization/TransformOps/BufferizationTransformOps.h"
+#include "mlir/Dialect/EmitC/TransformOps/EmitCTransformOps.h"
#include "mlir/Dialect/Func/Extensions/AllExtensions.h"
#include "mlir/Dialect/Func/TransformOps/FuncTransformOps.h"
#include "mlir/Dialect/GPU/TransformOps/GPUTransformOps.h"
@@ -67,6 +68,7 @@ inline void registerAllExtensions(DialectRegistry ®istry) {
// Register all transform dialect extensions.
affine::registerTransformDialectExtension(registry);
bufferization::registerTransformDialectExtension(registry);
+ emitc::registerTransformDialectExtension(registry);
func::registerTransformDialectExtension(registry);
gpu::registerTransformDialectExtension(registry);
linalg::registerTransformDialectExtension(registry);
diff --git a/mlir/include/mlir/InitAllPasses.h b/mlir/include/mlir/InitAllPasses.h
index f22980036ffcfa1..5207559f3625095 100644
--- a/mlir/include/mlir/InitAllPasses.h
+++ b/mlir/include/mlir/InitAllPasses.h
@@ -23,6 +23,7 @@
#include "mlir/Dialect/Async/Passes.h"
#include "mlir/Dialect/Bufferization/Pipelines/Passes.h"
#include "mlir/Dialect/Bufferization/Transforms/Passes.h"
+#include "mlir/Dialect/EmitC/Transforms/Passes.h"
#include "mlir/Dialect/Func/Transforms/Passes.h"
#include "mlir/Dialect/GPU/Transforms/Passes.h"
#include "mlir/Dialect/LLVMIR/Transforms/Passes.h"
@@ -86,6 +87,7 @@ inline void registerAllPasses() {
vector::registerVectorPasses();
arm_sme::registerArmSMEPasses();
arm_sve::registerArmSVEPasses();
+ emitc::registerEmitCPasses();
// Dialect pipelines
bufferization::registerBufferizationPipelines();
diff --git a/mlir/lib/Dialect/EmitC/CMakeLists.txt b/mlir/lib/Dialect/EmitC/CMakeLists.txt
index f33061b2d87cffc..660deb21479d297 100644
--- a/mlir/lib/Dialect/EmitC/CMakeLists.txt
+++ b/mlir/lib/Dialect/EmitC/CMakeLists.txt
@@ -1 +1,3 @@
add_subdirectory(IR)
+add_subdirectory(TransformOps)
+add_subdirectory(Transforms)
diff --git a/mlir/lib/Dialect/EmitC/IR/EmitC.cpp b/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
index d06381b7ddad3dc..26599f62680319f 100644
--- a/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
+++ b/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
@@ -189,6 +189,82 @@ LogicalResult emitc::ConstantOp::verify() {
OpFoldResult emitc::ConstantOp::fold(FoldAdaptor adaptor) { return getValue(); }
+//===----------------------------------------------------------------------===//
+// ExpressionOp
+//===----------------------------------------------------------------------===//
+
+Operation *ExpressionOp::getRootOp() {
+ auto yieldOp = cast<YieldOp>(getRegion().front().getTerminator());
+ Value yieldedValue = yieldOp.getResult();
+ Operation *rootOp = yieldedValue.getDefiningOp();
+ assert(rootOp && "Yielded value not defined within expression");
+ return rootOp;
+}
+
+ParseResult ExpressionOp::parse(OpAsmParser &parser, OperationState &result) {
+ // Parse the optional attribute list.
+ if (parser.parseOptionalAttrDict(result.attributes))
+ return failure();
+
+ // Parse results type.
+ Type expressionType;
+ if (parser.parseColonType(expressionType))
+ return failure();
+ result.addTypes(expressionType);
+
+ // Create the expression's body region.
+ result.regions.reserve(1);
+ Region *region = result.addRegion();
+
+ // Parse the region.
+ if (parser.parseRegion(*region, /*arguments=*/{}, /*argTypes=*/{}))
+ return failure();
+
+ return success();
+}
+
+void ExpressionOp::print(OpAsmPrinter &p) {
+ p.printOptionalAttrDict((*this)->getAttrs());
+
+ p << " : " << getResult().getType() << ' ';
+
+ p.printRegion(getRegion(),
+ /*printEntryBlockArgs=*/false,
+ /*printBlockTerminators=*/true);
+}
+
+LogicalResult ExpressionOp::verify() {
+ Type resultType = getResult().getType();
+ Region ®ion = getRegion();
+
+ Block &body = region.front();
+
+ if (!body.mightHaveTerminator())
+ return emitOpError("must yield a value at termination");
+
+ auto yield = cast<YieldOp>(body.getTerminator());
+ Value yieldResult = yield.getResult();
+
+ if (!yieldResult)
+ return emitOpError("must yield a value at termination");
+
+ Type yieldType = yieldResult.getType();
+
+ if (resultType != yieldType)
+ return emitOpError("requires yielded type to match return type");
+
+ for (Operation &op : region.front().without_terminator()) {
+ if (!isCExpression(op))
+ return emitOpError("contains an unsupported operation");
+ if (op.getNumResults() !...
[truncated]
|
mlir/include/mlir/Dialect/EmitC/TransformOps/EmitCTransformOps.h
Outdated
Show resolved
Hide resolved
mlir/include/mlir/Dialect/EmitC/TransformOps/EmitCTransformOps.td
Outdated
Show resolved
Hide resolved
✅ With the latest revision this PR passed the C/C++ code formatter. |
Thanks for the PR Gil, this looks great. I would like to go over the changes in the emitter one last time, as these are the most invasive. Furthermore I can't comment on the transform dialect stuff, as I have no experience with this. But the lit tests look reasonable to me. |
Thanks @simon-camp !
@ftynse , could you please take a look at the transform dialect part? |
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.
With #72494 landed this needs to be rebased. Otherwise this looks good to me. I would still like someone to look over the transform dialect stuff though.
Looks good from my side as well, but I am also not familar with the transform dialect related parts. As @aniragil already asked @ftynse to take a look, I requested a review from him. |
90f53aa
to
046c837
Compare
046c837
to
dc7a02b
Compare
Removed transforms part, will be uploaded separately as a follow-up PR. |
80c890c
to
52074ba
Compare
Excuse the delayed reply, just returning from out-of-office. Despite your comment, it seems that the transforms part is still part of the patch? |
Sorry, should have been more clear: The transform-dialect related parts were taken out. The transformations themselves are still there as patterns available as a pass. The patch follows the naming convention of placing concrete transformation code under /Transforms and the transform ops under /TransformOps. |
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.
Only some minor remarks and no comment from my side that would prevent landing your work. Anyway, I would assume that it would be good if @simon-camp takes another looking before landing this.
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 besides the formatting mentioned by @marbre
Add an emitc.expression operation that models C expressions, and provide a pass to form and fold EmitC ops into expressions. The translator emits the body of emitc.expression ops as a single C expression. This expression is emitted by default as the RHS of an EmitC SSA value, but if possible, expressions with a single use that is not another expression are instead inlined. Specific expression's inlining can be fine tuned by lowering passes and transforms.
fd93ee2
to
a252935
Compare
Add an emitc.expression operation that models C expressions, and provide transforms to form and fold expressions. The translator emits the body of emitc.expression ops as a single C expression. This expression is emitted by default as the RHS of an EmitC SSA value, but if possible, expressions with a single use that is not another expression are instead inlined. Specific expression's inlining can be fine tuned by lowering passes and transforms.
Add an emitc.expression operation that models C expressions, and provide
transforms to form and fold expressions. The translator emits the body of
emitc.expression ops as a single C expression.
This expression is emitted by default as the RHS of an EmitC SSA value, but if
possible, expressions with a single use that is not another expression are
instead inlined. Specific expression's inlining can be fine tuned by lowering
passes and transforms.