Skip to content

[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

Merged
merged 1 commit into from
Dec 20, 2023

Conversation

aniragil
Copy link
Contributor

@aniragil aniragil commented Nov 8, 2023

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.

@llvmbot
Copy link
Member

llvmbot commented Nov 8, 2023

@llvm/pr-subscribers-mlir-emitc

@llvm/pr-subscribers-mlir

Author: Gil Rapaport (aniragil)

Changes

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.


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:

  • (modified) mlir/include/mlir/Dialect/EmitC/CMakeLists.txt (+2)
  • (modified) mlir/include/mlir/Dialect/EmitC/IR/EmitC.td (+92-6)
  • (added) mlir/include/mlir/Dialect/EmitC/TransformOps/CMakeLists.txt (+6)
  • (added) mlir/include/mlir/Dialect/EmitC/TransformOps/EmitCTransformOps.h (+49)
  • (added) mlir/include/mlir/Dialect/EmitC/TransformOps/EmitCTransformOps.td (+70)
  • (added) mlir/include/mlir/Dialect/EmitC/Transforms/CMakeLists.txt (+5)
  • (added) mlir/include/mlir/Dialect/EmitC/Transforms/Passes.h (+35)
  • (added) mlir/include/mlir/Dialect/EmitC/Transforms/Passes.td (+24)
  • (added) mlir/include/mlir/Dialect/EmitC/Transforms/Transforms.h (+34)
  • (modified) mlir/include/mlir/InitAllExtensions.h (+2)
  • (modified) mlir/include/mlir/InitAllPasses.h (+2)
  • (modified) mlir/lib/Dialect/EmitC/CMakeLists.txt (+2)
  • (modified) mlir/lib/Dialect/EmitC/IR/EmitC.cpp (+93)
  • (added) mlir/lib/Dialect/EmitC/TransformOps/CMakeLists.txt (+15)
  • (added) mlir/lib/Dialect/EmitC/TransformOps/EmitCTransformOps.cpp (+114)
  • (added) mlir/lib/Dialect/EmitC/Transforms/CMakeLists.txt (+16)
  • (added) mlir/lib/Dialect/EmitC/Transforms/FormExpressions.cpp (+60)
  • (added) mlir/lib/Dialect/EmitC/Transforms/Transforms.cpp (+117)
  • (modified) mlir/lib/Target/Cpp/TranslateToCpp.cpp (+226-24)
  • (modified) mlir/test/Dialect/EmitC/invalid_ops.mlir (+58-1)
  • (modified) mlir/test/Dialect/EmitC/ops.mlir (+17)
  • (added) mlir/test/Dialect/EmitC/transform-ops.mlir (+131)
  • (added) mlir/test/Target/Cpp/expressions.mlir (+212)
  • (modified) mlir/test/Target/Cpp/for.mlir (+17-5)
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 &registry);
+} // 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 &registry);
+} // 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 &registry) {
   // 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 &region = 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]

@aniragil aniragil requested review from marbre and jpienaar November 8, 2023 06:57
@aniragil
Copy link
Contributor Author

aniragil commented Nov 8, 2023

@simon-camp
@ayalz

Copy link

github-actions bot commented Nov 8, 2023

✅ With the latest revision this PR passed the C/C++ code formatter.

@simon-camp
Copy link
Contributor

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.

@aniragil
Copy link
Contributor Author

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.

Thanks @simon-camp !

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.

@ftynse , could you please take a look at the transform dialect part?

Copy link
Contributor

@simon-camp simon-camp left a 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.

@marbre marbre requested a review from ftynse November 17, 2023 10:36
@marbre
Copy link
Member

marbre commented Nov 17, 2023

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.

@aniragil aniragil force-pushed the emitc-add-expressions branch 2 times, most recently from 90f53aa to 046c837 Compare November 25, 2023 12:44
@aniragil aniragil force-pushed the emitc-add-expressions branch from 046c837 to dc7a02b Compare December 5, 2023 13:57
@aniragil aniragil removed the request for review from ftynse December 5, 2023 13:58
@aniragil
Copy link
Contributor Author

aniragil commented Dec 5, 2023

Removed transforms part, will be uploaded separately as a follow-up PR.

@aniragil aniragil requested review from simon-camp and marbre December 5, 2023 14:02
@aniragil aniragil force-pushed the emitc-add-expressions branch 4 times, most recently from 80c890c to 52074ba Compare December 12, 2023 11:09
@marbre
Copy link
Member

marbre commented Dec 12, 2023

Removed transforms part, will be uploaded separately as a follow-up PR.

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?

@aniragil
Copy link
Contributor Author

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.

Copy link
Member

@marbre marbre left a 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.

Copy link
Contributor

@simon-camp simon-camp left a 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.
@aniragil aniragil force-pushed the emitc-add-expressions branch from fd93ee2 to a252935 Compare December 20, 2023 10:06
@aniragil aniragil merged commit d980384 into llvm:main Dec 20, 2023
mgehre-amd pushed a commit to Xilinx/llvm-project that referenced this pull request Mar 11, 2024
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants