Add a structured if operation #67234
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-mlir @llvm/pr-subscribers-mlir-emitc ChangesAdd an emitc.if op to the EmitC dialect. A new convert-scf-to-emitc The emitc.if op doesn't return any value and its then/else regions are Patch is 34.58 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/67234.diff 14 Files Affected:
diff --git a/mlir/include/mlir/Conversion/Passes.h b/mlir/include/mlir/Conversion/Passes.h
index fc5e9adba114405..41806004fc1dca8 100644
--- a/mlir/include/mlir/Conversion/Passes.h
+++ b/mlir/include/mlir/Conversion/Passes.h
@@ -48,6 +48,7 @@
#include "mlir/Conversion/PDLToPDLInterp/PDLToPDLInterp.h"
#include "mlir/Conversion/ReconcileUnrealizedCasts/ReconcileUnrealizedCasts.h"
#include "mlir/Conversion/SCFToControlFlow/SCFToControlFlow.h"
+#include "mlir/Conversion/SCFToEmitC/SCFToEmitC.h"
#include "mlir/Conversion/SCFToGPU/SCFToGPUPass.h"
#include "mlir/Conversion/SCFToOpenMP/SCFToOpenMP.h"
#include "mlir/Conversion/SCFToSPIRV/SCFToSPIRVPass.h"
diff --git a/mlir/include/mlir/Conversion/Passes.td b/mlir/include/mlir/Conversion/Passes.td
index 11008baa0160efe..cca1e262df7c121 100644
--- a/mlir/include/mlir/Conversion/Passes.td
+++ b/mlir/include/mlir/Conversion/Passes.td
@@ -931,6 +931,17 @@ def ConvertParallelLoopToGpu : Pass<"convert-parallel-loops-to-gpu"> {
let dependentDialects = ["affine::AffineDialect", "gpu::GPUDialect"];
}
+//===----------------------------------------------------------------------===//
+// SCFToEmitC
+//===----------------------------------------------------------------------===//
+
+def SCFToEmitC : Pass<"convert-scf-to-emitc"> {
+ let summary = "Convert SCF dialect to EmitC dialect, maintaining structured"
+ " control flow";
+ let constructor = "mlir::createConvertSCFToEmitCPass()";
+ let dependentDialects = ["emitc::EmitCDialect"];
+}
+
//===----------------------------------------------------------------------===//
// ShapeToStandard
//===----------------------------------------------------------------------===//
diff --git a/mlir/include/mlir/Conversion/SCFToEmitC/SCFToEmitC.h b/mlir/include/mlir/Conversion/SCFToEmitC/SCFToEmitC.h
new file mode 100644
index 000000000000000..ec7a9c5de634496
--- /dev/null
+++ b/mlir/include/mlir/Conversion/SCFToEmitC/SCFToEmitC.h
@@ -0,0 +1,29 @@
+//===- SCFToEmitC.h - SCF to EmitC Pass entrypoint --*- 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_SCFTOEMITC_SCFTOEMITC_H_
+#define MLIR_CONVERSION_SCFTOEMITC_SCFTOEMITC_H_
+
+#include <memory>
+
+namespace mlir {
+class Pass;
+class RewritePatternSet;
+
+#define GEN_PASS_DECL_SCFTOEMITC
+#include "mlir/Conversion/Passes.h.inc"
+
+/// Collect a set of patterns to convert SCF operations to the EmitC dialect.
+void populateSCFToEmitCConversionPatterns(RewritePatternSet &patterns);
+
+/// Creates a pass to convert SCF operations to the EmitC dialect.
+std::unique_ptr<Pass> createConvertSCFToEmitCPass();
+
+} // namespace mlir
+
+#endif // MLIR_CONVERSION_SCFTOEMITC_SCFTOEMITC_H_
diff --git a/mlir/include/mlir/Dialect/EmitC/IR/EmitC.h b/mlir/include/mlir/Dialect/EmitC/IR/EmitC.h
index b3c1170eefdab90..4dff26e23c42850 100644
--- a/mlir/include/mlir/Dialect/EmitC/IR/EmitC.h
+++ b/mlir/include/mlir/Dialect/EmitC/IR/EmitC.h
@@ -14,15 +14,23 @@
#define MLIR_DIALECT_EMITC_IR_EMITC_H
#include "mlir/Bytecode/BytecodeOpInterface.h"
+#include "mlir/IR/Builders.h"
#include "mlir/IR/BuiltinOps.h"
#include "mlir/IR/BuiltinTypes.h"
#include "mlir/IR/Dialect.h"
#include "mlir/Interfaces/CastInterfaces.h"
+#include "mlir/Interfaces/ControlFlowInterfaces.h"
#include "mlir/Interfaces/SideEffectInterfaces.h"
#include "mlir/Dialect/EmitC/IR/EmitCDialect.h.inc"
#include "mlir/Dialect/EmitC/IR/EmitCEnums.h.inc"
+namespace mlir {
+namespace emitc {
+void buildTerminatedBody(OpBuilder &builder, Location loc);
+} // namespace emitc
+} // namespace mlir
+
#define GET_ATTRDEF_CLASSES
#include "mlir/Dialect/EmitC/IR/EmitCAttributes.h.inc"
diff --git a/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td b/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td
index 40a4896aca4b633..ad28763c89f67a5 100644
--- a/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td
+++ b/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td
@@ -17,6 +17,7 @@ include "mlir/Dialect/EmitC/IR/EmitCAttributes.td"
include "mlir/Dialect/EmitC/IR/EmitCTypes.td"
include "mlir/Interfaces/CastInterfaces.td"
+include "mlir/Interfaces/ControlFlowInterfaces.td"
include "mlir/Interfaces/SideEffectInterfaces.td"
//===----------------------------------------------------------------------===//
@@ -402,4 +403,103 @@ def EmitC_VariableOp : EmitC_Op<"variable", []> {
let hasVerifier = 1;
}
+def EmitC_AssignOp : EmitC_Op<"assign", [MemoryEffects<[MemWrite]>]> {
+ let summary = "Assign operation";
+ let description = [{
+ The `assign` operation stores an SSA value to the location designated by an
+ EmitC variable. This operation doesn't return any value. The assigned value
+ must be of the same type as the variable being assigned. The operation is
+ emitted as a C/C++ '=' operator.
+
+ Example:
+
+ ```mlir
+ // Integer variable
+ %0 = "emitc.variable"(){value = 42 : i32} : () -> i32
+ %1 = emitc.call "foo"() : () -> (i32)
+
+ // Assign emitted as `... = ...;`
+ "emitc.assign"(%0, %1) : (i32, %i32) -> ()
+ ```
+ }];
+
+ let arguments = (ins AnyType:$var, AnyType:$value);
+ let results = (outs);
+
+ let hasVerifier = 1;
+ let assemblyFormat = "$value `:` type($value) `to` $var `:` type($var) attr-dict";
+}
+
+def YieldOp : EmitC_Op<"yield", [Pure, Terminator, ParentOneOf<["IfOp"]>]> {
+ 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.
+ }];
+
+ let arguments = (ins);
+ let builders = [OpBuilder<(ins), [{ /* nothing to do */ }]>];
+
+ let assemblyFormat = [{ attr-dict }];
+}
+
+def EmitC_IfOp : EmitC_Op<"if",
+ [DeclareOpInterfaceMethods<RegionBranchOpInterface, [
+ "getNumRegionInvocations", "getRegionInvocationBounds",
+ "getEntrySuccessorRegions"]>, SingleBlock,
+ SingleBlockImplicitTerminator<"emitc::YieldOp">,
+ RecursiveMemoryEffects, NoRegionArguments]> {
+ let summary = "if-then-else operation";
+ let description = [{
+ The `if` operation represents an if-then-else construct for
+ conditionally executing two regions of code. The operand to an if operation
+ is a boolean value. For example:
+
+ ```mlir
+ emitc.if %b {
+ ...
+ } else {
+ ...
+ }
+ ```
+
+ The "then" region has exactly 1 block. The "else" region may have 0 or 1
+ blocks. The blocks are always terminated with `emitc.yield`, which can be
+ left out to be inserted implicitly. This operation doesn't produce any
+ results.
+ }];
+ let arguments = (ins I1:$condition);
+ let results = (outs);
+ let regions = (region SizedRegion<1>:$thenRegion,
+ MaxSizedRegion<1>:$elseRegion);
+
+ let skipDefaultBuilders = 1;
+ let builders = [
+ OpBuilder<(ins "Value":$cond)>,
+ OpBuilder<(ins "Value":$cond, "bool":$addThenBlock, "bool":$addElseBlock)>,
+ OpBuilder<(ins "Value":$cond, "bool":$withElseRegion)>,
+ OpBuilder<(ins "Value":$cond, "bool":$withElseRegion)>,
+ OpBuilder<(ins "Value":$cond,
+ CArg<"function_ref<void(OpBuilder &, Location)>",
+ "buildTerminatedBody">:$thenBuilder,
+ CArg<"function_ref<void(OpBuilder &, Location)>",
+ "nullptr">:$elseBuilder)>,
+ ];
+
+ let extraClassDeclaration = [{
+ OpBuilder getThenBodyBuilder(OpBuilder::Listener *listener = nullptr) {
+ Block* body = getBody(0);
+ return OpBuilder::atBlockEnd(body, listener);
+ }
+ OpBuilder getElseBodyBuilder(OpBuilder::Listener *listener = nullptr) {
+ Block* body = getBody(1);
+ return OpBuilder::atBlockEnd(body, listener);
+ }
+ Block* thenBlock();
+ Block* elseBlock();
+ }];
+ let hasCustomAssemblyFormat = 1;
+}
+
#endif // MLIR_DIALECT_EMITC_IR_EMITC
diff --git a/mlir/lib/Conversion/CMakeLists.txt b/mlir/lib/Conversion/CMakeLists.txt
index 275e095245e89ce..660e48768c4ff34 100644
--- a/mlir/lib/Conversion/CMakeLists.txt
+++ b/mlir/lib/Conversion/CMakeLists.txt
@@ -38,6 +38,7 @@ add_subdirectory(OpenMPToLLVM)
add_subdirectory(PDLToPDLInterp)
add_subdirectory(ReconcileUnrealizedCasts)
add_subdirectory(SCFToControlFlow)
+add_subdirectory(SCFToEmitC)
add_subdirectory(SCFToGPU)
add_subdirectory(SCFToOpenMP)
add_subdirectory(SCFToSPIRV)
diff --git a/mlir/lib/Conversion/SCFToEmitC/CMakeLists.txt b/mlir/lib/Conversion/SCFToEmitC/CMakeLists.txt
new file mode 100644
index 000000000000000..79119d374f7a5e9
--- /dev/null
+++ b/mlir/lib/Conversion/SCFToEmitC/CMakeLists.txt
@@ -0,0 +1,18 @@
+add_mlir_conversion_library(MLIRSCFToEmitC
+ SCFToEmitC.cpp
+
+ ADDITIONAL_HEADER_DIRS
+ ${MLIR_MAIN_INCLUDE_DIR}/mlir/Conversion/SCFToEmitC
+
+ DEPENDS
+ MLIRConversionPassIncGen
+
+ LINK_COMPONENTS
+ Core
+
+ LINK_LIBS PUBLIC
+ MLIRArithDialect
+ MLIREmitCDialect
+ MLIRSCFDialect
+ MLIRTransforms
+ )
diff --git a/mlir/lib/Conversion/SCFToEmitC/SCFToEmitC.cpp b/mlir/lib/Conversion/SCFToEmitC/SCFToEmitC.cpp
new file mode 100644
index 000000000000000..1fa518bad7cfb29
--- /dev/null
+++ b/mlir/lib/Conversion/SCFToEmitC/SCFToEmitC.cpp
@@ -0,0 +1,131 @@
+//===- SCFToEmitC.cpp - SCF to CF 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
+//
+//===----------------------------------------------------------------------===//
+//
+// This file implements a pass to convert scf.for, scf.if and loop.terminator
+// ops into standard CFG ops.
+//
+//===----------------------------------------------------------------------===//
+
+#include "mlir/Conversion/SCFToEmitC/SCFToEmitC.h"
+
+#include "mlir/Dialect/Arith/IR/Arith.h"
+#include "mlir/Dialect/EmitC/IR/EmitC.h"
+#include "mlir/Dialect/SCF/IR/SCF.h"
+#include "mlir/IR/Builders.h"
+#include "mlir/IR/BuiltinOps.h"
+#include "mlir/IR/IRMapping.h"
+#include "mlir/IR/MLIRContext.h"
+#include "mlir/IR/PatternMatch.h"
+#include "mlir/Transforms/DialectConversion.h"
+#include "mlir/Transforms/Passes.h"
+
+namespace mlir {
+#define GEN_PASS_DEF_SCFTOEMITC
+#include "mlir/Conversion/Passes.h.inc"
+} // namespace mlir
+
+using namespace mlir;
+using namespace mlir::scf;
+
+namespace {
+
+struct SCFToEmitCPass : public impl::SCFToEmitCBase<SCFToEmitCPass> {
+ void runOnOperation() override;
+};
+
+// Lower scf::if to emitc::if, implementing return values as emitc::variable's
+// updated within the then and else regions.
+struct IfLowering : public OpRewritePattern<IfOp> {
+ using OpRewritePattern<IfOp>::OpRewritePattern;
+
+ LogicalResult matchAndRewrite(IfOp ifOp,
+ PatternRewriter &rewriter) const override;
+};
+
+} // namespace
+
+LogicalResult IfLowering::matchAndRewrite(IfOp ifOp,
+ PatternRewriter &rewriter) const {
+ auto loc = ifOp.getLoc();
+
+ SmallVector<Value> resultVariables;
+
+ // Create an emitc::variable op for each result. These variables will be
+ // assigned to by emitc::assign ops within the then & else regions.
+ if (ifOp.getNumResults()) {
+ auto context = ifOp.getContext();
+ rewriter.setInsertionPoint(ifOp);
+ for (auto result : ifOp.getResults()) {
+ auto resultType = result.getType();
+ auto noInit = emitc::OpaqueAttr::get(context, "");
+ auto var = rewriter.create<emitc::VariableOp>(loc, resultType, noInit);
+ resultVariables.push_back(var);
+ }
+ }
+
+ // Utility function to lower the contents of an scf::if region to an emitc::if
+ // region. The contents of the scf::if regions is moved into the respective
+ // emitc::if regions, but the scf::yield is replaced not only with an
+ // emitc::yield, but also with a sequence of emitc::assign ops that set the
+ // yielded values into the result variables.
+ auto lowerRegion = [&resultVariables, &rewriter](Region ®ion,
+ Region &loweredRegion) {
+ rewriter.inlineRegionBefore(region, loweredRegion, loweredRegion.end());
+ Operation *terminator = loweredRegion.back().getTerminator();
+ auto terminatorLoc = terminator->getLoc();
+ ValueRange terminatorOperands = terminator->getOperands();
+ rewriter.setInsertionPointToEnd(&loweredRegion.back());
+ for (auto value2Var : llvm::zip(terminatorOperands, resultVariables)) {
+ auto resultValue = std::get<0>(value2Var);
+ auto resultVar = std::get<1>(value2Var);
+ rewriter.create<emitc::AssignOp>(terminatorLoc, resultVar, resultValue);
+ }
+ rewriter.create<emitc::YieldOp>(terminatorLoc);
+ rewriter.eraseOp(terminator);
+ };
+
+ auto &thenRegion = ifOp.getThenRegion();
+ auto &elseRegion = ifOp.getElseRegion();
+
+ bool hasElseBlock = !elseRegion.empty();
+
+ auto loweredIf =
+ rewriter.create<emitc::IfOp>(loc, ifOp.getCondition(), false, false);
+
+ auto &loweredThenRegion = loweredIf.getThenRegion();
+ lowerRegion(thenRegion, loweredThenRegion);
+
+ if (hasElseBlock) {
+ auto &loweredElseRegion = loweredIf.getElseRegion();
+ lowerRegion(elseRegion, loweredElseRegion);
+ }
+
+ rewriter.replaceOp(ifOp, resultVariables);
+ return success();
+}
+
+void mlir::populateSCFToEmitCConversionPatterns(RewritePatternSet &patterns) {
+ patterns.add<IfLowering>(patterns.getContext());
+}
+
+void SCFToEmitCPass::runOnOperation() {
+ RewritePatternSet patterns(&getContext());
+ populateSCFToEmitCConversionPatterns(patterns);
+
+ // Configure conversion to lower out SCF operations.
+ ConversionTarget target(getContext());
+ target.addIllegalOp<scf::IfOp>();
+ target.markUnknownOpDynamicallyLegal([](Operation *) { return true; });
+ if (failed(
+ applyPartialConversion(getOperation(), target, std::move(patterns))))
+ signalPassFailure();
+}
+
+std::unique_ptr<Pass> mlir::createConvertSCFToEmitCPass() {
+ return std::make_unique<SCFToEmitCPass>();
+}
diff --git a/mlir/lib/Dialect/EmitC/IR/EmitC.cpp b/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
index 1c95ed07702d1db..447c7b748a90015 100644
--- a/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
+++ b/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
@@ -44,6 +44,12 @@ Operation *EmitCDialect::materializeConstant(OpBuilder &builder,
return builder.create<emitc::ConstantOp>(loc, type, value);
}
+/// Default callback for builders of ops carrying a region. Inserts a yield
+/// without arguments.
+void mlir::emitc::buildTerminatedBody(OpBuilder &builder, Location loc) {
+ builder.create<emitc::YieldOp>(loc);
+}
+
//===----------------------------------------------------------------------===//
// AddOp
//===----------------------------------------------------------------------===//
@@ -248,6 +254,187 @@ LogicalResult emitc::VariableOp::verify() {
return success();
}
+//===----------------------------------------------------------------------===//
+// AssignOp
+//===----------------------------------------------------------------------===//
+
+/// The assign op requires that the assigned value's type matches the
+/// assigned-to variable type.
+LogicalResult emitc::AssignOp::verify() {
+ auto variable = getVar();
+ auto variableDef = variable.getDefiningOp();
+ if (!variableDef || !llvm::isa<emitc::VariableOp>(variableDef))
+ return emitOpError() << "requires first operand (" << variable
+ << ") to be a Variable";
+
+ auto value = getValue();
+ if (variable.getType() != value.getType())
+ return emitOpError() << "requires value's type (" << value.getType()
+ << ") to match variable's type (" << variable.getType()
+ << ")";
+ return success();
+}
+
+//===----------------------------------------------------------------------===//
+// IfOp
+//===----------------------------------------------------------------------===//
+
+void IfOp::build(OpBuilder &builder, OperationState &result, Value cond,
+ bool addThenBlock, bool addElseBlock) {
+ assert((!addElseBlock || addThenBlock) &&
+ "must not create else block w/o then block");
+ result.addOperands(cond);
+
+ // Add regions and blocks.
+ OpBuilder::InsertionGuard guard(builder);
+ Region *thenRegion = result.addRegion();
+ if (addThenBlock)
+ builder.createBlock(thenRegion);
+ Region *elseRegion = result.addRegion();
+ if (addElseBlock)
+ builder.createBlock(elseRegion);
+}
+
+void IfOp::build(OpBuilder &builder, OperationState &result, Value cond,
+ bool withElseRegion) {
+ result.addOperands(cond);
+
+ // Build then region.
+ OpBuilder::InsertionGuard guard(builder);
+ Region *thenRegion = result.addRegion();
+ builder.createBlock(thenRegion);
+
+ // Build else region.
+ Region *elseRegion = result.addRegion();
+ if (withElseRegion) {
+ builder.createBlock(elseRegion);
+ }
+}
+
+void IfOp::build(OpBuilder &builder, OperationState &result, Value cond,
+ function_ref<void(OpBuilder &, Location)> thenBuilder,
+ function_ref<void(OpBuilder &, Location)> elseBuilder) {
+ assert(thenBuilder && "the builder callback for 'then' must be present");
+ result.addOperands(cond);
+
+ // Build then region.
+ OpBuilder::InsertionGuard guard(builder);
+ Region *thenRegion = result.addRegion();
+ builder.createBlock(thenRegion);
+ thenBuilder(builder, result.location);
+
+ // Build else region.
+ Region *elseRegion = result.addRegion();
+ if (elseBuilder) {
+ builder.createBlock(elseRegion);
+ elseBuilder(builder, result.location);
+ }
+}
+
+ParseResult IfOp::parse(OpAsmParser &parser, OperationState &result) {
+ // Create the regions for 'then'.
+ result.regions.reserve(2);
+ Region *thenRegion = result.addRegion();
+ Region *elseRegion = result.addRegion();
+
+ auto &builder = parser.getBuilder();
+ OpAsmParser::UnresolvedOperand cond;
+ Type i1Type = builder.getIntegerType(1);
+ if (parser.parseOperand(cond) ||
+ parser.resolveOperand(cond, i1Type, result.operands))
+ return failure();
+ // Parse the 'then' region.
+ if (parser.parseRegion(*thenRegion, /*arguments=*/{}, /*argTypes=*/{}))
+ return failure();
+ IfOp::ensureTerminator(*thenRegion, parser.getBuilder(), result.location);
+
+ // If we find an 'else' keyword then parse the 'else' region.
+ if (!parser.parseOptionalKeyword("else")) {
+ if (parser.parseRegion(*elseRegion, /*arguments=*/{}, /*argTypes=*/{}))
+ return failure();
+ IfOp::ensureTerminator(*elseRegion, parser.getBuilder(), result.location);
+ }
+
+ // Parse the optional attribute list.
+ if (parser.parseOptionalAttrDict(result.attributes))
+ return failure();
+ return success();
+}
+
+void IfOp::print(OpAsmPrinter &p) {
+ bool printBlockTerminators = false;
+
+ p << " " << getCondition();
+ p << ' ';
+ p.printRegion(getThenRegion(),
+ /*printEntryBlockArgs=*/false,
+ /*printBlockTerminators=*/printBlockTerminators);
+
+ // Print the 'else' regions if it exists and has a block.
+ auto &elseRegion = getElseRegion();
+ if (!elseRegion.empty()) {
+ p << " else ";
+ p.printRegion(elseRegion,
+ /*printEntryBlockArgs=*/false,
+ /*printBlockTerminators=*/printBlockTerminators);
+ }
+
+ p.printOptionalAttrDict((*this)->getAttrs());
+}
+
+/// Given the region at `index`, or the parent operation if `index` is None,
+/// return the successor regions. These are the regions that may be selected
+/// during the flow of control. `operands` is a set of optional attributes that
+/// correspond to a constant value for each operand, or null if that operand is
+/// not a constant.
+void IfOp::getSuccessorRegions(RegionBranchPoint point,
+ SmallVectorImpl<RegionSuccessor> ®ions) {
+ // The `then` and the `else` region branch back to the parent operation.
+ if (!point.isParent()) {
+ regions.push_back(RegionSuccessor());
+ return;
+ }
+
+ regions.push_back(RegionSuccessor(&getThenRegion()));
+
+ // Don't consider the else region if it is empty.
+ Region *elseRegion = &this->getElseRegion();
+ if (elseRegion->empty()...
[truncated]
|
aniragil
force-pushed
the
emitc-add-if-op
branch
from
cccfd25 to
24b1900
Compare
Thanks for taking a look, Mehdi! (To bring others on the same page: this patch is based on a discussion Mehdi, Jacques and I had at the Edinburgh hackathon on bringing the EmitC translator to handle only the EmitC dialect, simplifying it in the process. This patch starts replacing SCF translation with SCF-to-EmitC lowering) |
aniragil
force-pushed
the
emitc-add-if-op
branch
from
b1d2aa0 to
a65a277
Compare
|
Thanks for the PR @aniragil. I love to see that this seems to move work previously handled in the emitter to the dialect and dedicated conversions. We initially had an Unfortunately, I am currently on a business travel and have only very little time to review. @simon-camp can you already take a first look? |
simon-camp
reviewed
Sep 25, 2023
Should all be fixed. Thanks Simon! |
marbre
reviewed
Sep 25, 2023
There was a problem hiding this comment.
Had a chance to take a short look (expect for the conversion). Overall looks good to me. One thing I noticed is that autois used quite often, while the LLVM guidelines ask for a more moderate use, see https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable and https://llvm.org/docs/CodingStandards.html#beware-unnecessary-copies-with-auto.
Furthermore, you could delete
llvm-project/mlir/docs/Dialects/emitc.md
Line 36 in d3505c2
Indeed ... fixing.
Ah, missed the doc. Done. Thanks Marius! |
simon-camp
reviewed
Sep 26, 2023
marbre
approved these changes
Sep 27, 2023
Add an emitc.if op to the EmitC dialect. A new convert-scf-to-emitc pass replaces the existing direct translation of scf.if to C; The translator now handles emitc.if instead. The emitc.if op doesn't return any value and its then/else regions are terminated with a new scf.yield op. Values returned by scf.if are lowered using emitc.variable ops, assigned to in the then/else regions using a new emitc.assign op.
aniragil
force-pushed
the
emitc-add-if-op
branch
from
3e3a6de to
06eb2b9
Compare
aniragil
added a commit
that referenced
this pull request
Sep 27, 2023
This reverts commit 126f037. Reverting due to bot failures.
|
I saw that you reverted the patch after I submitted a bazel build fix in 1fd2251. Will the patch be resubmitted? If so, I will leave the bazel fix in tree (assuming that you are not going to change the code structure dramatically). |
This should fix your failure. |
|
Yes, I'll resubmit with the missing library added to the CMakeLists.txt
…On Thu, Sep 28, 2023 at 1:00 AM Peiming Liu ***@***.***> wrote:
I saw that you reverted the patch after I submitted a bazel build fix in
1fd2251
<1fd2251>
.
Will the patch be resubmitted? If so, I will leave the bazel fix in tree
(assuming that you are not going to change the code structure dramatically).
—
Reply to this email directly, view it on GitHub
<#67234 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ANF3QVSV6SCQH3ZZDOHD3WLX4SOW3ANCNFSM6AAAAAA5EEBVRU>
.
You are receiving this because you modified the open/close state.Message
ID: ***@***.***>
|
Yes, resubmitting with the missing library dependence. |
Thanks Mehdi! |
aniragil
added a commit
that referenced
this pull request
Sep 27, 2023
legrosbuffle
pushed a commit
to legrosbuffle/llvm-project
that referenced
this pull request
Sep 29, 2023
Add an emitc.if op to the EmitC dialect. A new convert-scf-to-emitc pass replaces the existing direct translation of scf.if to C; The translator now handles emitc.if instead. The emitc.if op doesn't return any value and its then/else regions are terminated with a new scf.yield op. Values returned by scf.if are lowered using emitc.variable ops, assigned to in the then/else regions using a new emitc.assign op.
legrosbuffle
pushed a commit
to legrosbuffle/llvm-project
that referenced
this pull request
Sep 29, 2023
This reverts commit 126f037. Reverting due to bot failures.
legrosbuffle
pushed a commit
to legrosbuffle/llvm-project
that referenced
this pull request
Sep 29, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add an emitc.if op to the EmitC dialect. A new convert-scf-to-emitc
pass replaces the existing direct translation of scf.if to C; The
translator now handles emitc.if instead.
The emitc.if op doesn't return any value and its then/else regions are
terminated with a new scf.yield op. Values returned by scf.if are
lowered using emitc.variable ops, assigned to in the then/else regions
using a new emitc.assign op.