[Mlir-commits] [mlir] Revert "[mlir][emitc] Lower multiple results as a struct (#200659)" (PR #202911)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Wed Jun 10 02:59:34 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-mlir-emitc
Author: Gil Rapaport (aniragil)
<details>
<summary>Changes</summary>
This reverts commit 1e0a4c7a9154e46ef52a7c5b0ddbca69fbdcfacd.
Failed buildbot:
https://lab.llvm.org/buildbot/#/builders/116/builds/29302
---
Patch is 38.10 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/202911.diff
13 Files Affected:
- (modified) mlir/include/mlir/Conversion/ConvertToEmitC/ConvertToEmitCPatternInterface.td (+1-2)
- (modified) mlir/include/mlir/Conversion/ConvertToEmitC/ToEmitCInterface.h (-2)
- (modified) mlir/include/mlir/Conversion/FuncToEmitC/FuncToEmitC.h (+1-2)
- (modified) mlir/include/mlir/Conversion/Passes.td (-6)
- (modified) mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp (+1-1)
- (modified) mlir/lib/Conversion/ConvertToEmitC/ConvertToEmitCPass.cpp (+5-13)
- (modified) mlir/lib/Conversion/FuncToEmitC/FuncToEmitC.cpp (+25-236)
- (modified) mlir/lib/Conversion/FuncToEmitC/FuncToEmitCPass.cpp (+1-2)
- (modified) mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitC.cpp (+1-1)
- (modified) mlir/lib/Conversion/SCFToEmitC/SCFToEmitC.cpp (+1-1)
- (modified) mlir/test/Conversion/FuncToEmitC/func-to-emitc-failed.mlir (+1-87)
- (modified) mlir/test/Conversion/FuncToEmitC/func-to-emitc.mlir (+2-96)
- (modified) mlir/test/Target/Cpp/func.mlir (-63)
``````````diff
diff --git a/mlir/include/mlir/Conversion/ConvertToEmitC/ConvertToEmitCPatternInterface.td b/mlir/include/mlir/Conversion/ConvertToEmitC/ConvertToEmitCPatternInterface.td
index 9f494097e4251..e300826cdc281 100644
--- a/mlir/include/mlir/Conversion/ConvertToEmitC/ConvertToEmitCPatternInterface.td
+++ b/mlir/include/mlir/Conversion/ConvertToEmitC/ConvertToEmitCPatternInterface.td
@@ -14,8 +14,7 @@ def ConvertToEmitCPatternInterface : DialectInterface<"ConvertToEmitCPatternInte
}],
"void", "populateConvertToEmitCConversionPatterns",
(ins "::mlir::ConversionTarget &":$target, "::mlir::TypeConverter &":$typeConverter,
- "::mlir::RewritePatternSet &":$patterns,
- "::std::optional<bool>":$lowerToCpp)
+ "::mlir::RewritePatternSet &":$patterns)
>
];
}
diff --git a/mlir/include/mlir/Conversion/ConvertToEmitC/ToEmitCInterface.h b/mlir/include/mlir/Conversion/ConvertToEmitC/ToEmitCInterface.h
index 1532f89984ea7..f6546fc27e0b3 100644
--- a/mlir/include/mlir/Conversion/ConvertToEmitC/ToEmitCInterface.h
+++ b/mlir/include/mlir/Conversion/ConvertToEmitC/ToEmitCInterface.h
@@ -13,8 +13,6 @@
#include "mlir/IR/MLIRContext.h"
#include "mlir/IR/OpDefinition.h"
-#include <optional>
-
namespace mlir {
class ConversionTarget;
class TypeConverter;
diff --git a/mlir/include/mlir/Conversion/FuncToEmitC/FuncToEmitC.h b/mlir/include/mlir/Conversion/FuncToEmitC/FuncToEmitC.h
index 5e9e09bb14d74..be6b6cfe5a6db 100644
--- a/mlir/include/mlir/Conversion/FuncToEmitC/FuncToEmitC.h
+++ b/mlir/include/mlir/Conversion/FuncToEmitC/FuncToEmitC.h
@@ -15,8 +15,7 @@ class RewritePatternSet;
class TypeConverter;
void populateFuncToEmitCPatterns(const TypeConverter &typeConverter,
- RewritePatternSet &patterns,
- bool lowerToCpp = true);
+ RewritePatternSet &patterns);
void registerConvertFuncToEmitCInterface(DialectRegistry ®istry);
} // namespace mlir
diff --git a/mlir/include/mlir/Conversion/Passes.td b/mlir/include/mlir/Conversion/Passes.td
index 07e0e0c4e29e8..c30dd3b07d028 100644
--- a/mlir/include/mlir/Conversion/Passes.td
+++ b/mlir/include/mlir/Conversion/Passes.td
@@ -26,8 +26,6 @@ def ConvertToEmitC : Pass<"convert-to-emitc"> {
let options = [
ListOption<"filterDialects", "filter-dialects", "std::string",
"Test conversion patterns of only the specified dialects">,
- Option<"lowerToCpp", "lower-to-cpp", "bool", "",
- "Target C++ (true) instead of C (false)">,
];
}
@@ -455,10 +453,6 @@ def ConvertControlFlowToSPIRVPass : Pass<"convert-cf-to-spirv"> {
def ConvertFuncToEmitC : Pass<"convert-func-to-emitc", "ModuleOp"> {
let summary = "Convert Func dialect to EmitC dialect";
let dependentDialects = ["emitc::EmitCDialect"];
- let options = [Option<
- "lowerToCpp", "lower-to-cpp", "bool",
- /*default=*/"true",
- /*description=*/"Target C++ (true) instead of C (false)">];
}
//===----------------------------------------------------------------------===//
diff --git a/mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp b/mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp
index 5b074130925a4..d003dc7a6dff3 100644
--- a/mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp
+++ b/mlir/lib/Conversion/ArithToEmitC/ArithToEmitC.cpp
@@ -33,7 +33,7 @@ struct ArithToEmitCDialectInterface : public ConvertToEmitCPatternInterface {
/// and mark dialect legal for the conversion target.
void populateConvertToEmitCConversionPatterns(
ConversionTarget &target, TypeConverter &typeConverter,
- RewritePatternSet &patterns, std::optional<bool> lowerToCpp) const final {
+ RewritePatternSet &patterns) const final {
populateArithToEmitCPatterns(typeConverter, patterns);
}
};
diff --git a/mlir/lib/Conversion/ConvertToEmitC/ConvertToEmitCPass.cpp b/mlir/lib/Conversion/ConvertToEmitC/ConvertToEmitCPass.cpp
index 4f060eafa14cc..ee6d7d5e3c554 100644
--- a/mlir/lib/Conversion/ConvertToEmitC/ConvertToEmitCPass.cpp
+++ b/mlir/lib/Conversion/ConvertToEmitC/ConvertToEmitCPass.cpp
@@ -31,8 +31,7 @@ namespace {
class ConvertToEmitCPassInterface {
public:
ConvertToEmitCPassInterface(MLIRContext *context,
- ArrayRef<std::string> filterDialects,
- std::optional<bool> lowerToCpp);
+ ArrayRef<std::string> filterDialects);
virtual ~ConvertToEmitCPassInterface() = default;
/// Get the dependent dialects used by `convert-to-emitc`.
@@ -61,7 +60,6 @@ class ConvertToEmitCPassInterface {
MLIRContext *context;
/// List of dialects names to use as filters.
ArrayRef<std::string> filterDialects;
- std::optional<bool> lowerToCpp;
};
/// This DialectExtension can be attached to the context, which will invoke the
@@ -126,7 +124,7 @@ struct StaticConvertToEmitC : public ConvertToEmitCPassInterface {
// Populate the patterns with the dialect interface.
if (failed(visitInterfaces([&](ConvertToEmitCPatternInterface *iface) {
iface->populateConvertToEmitCConversionPatterns(
- *target, *typeConverter, tempPatterns, lowerToCpp);
+ *target, *typeConverter, tempPatterns);
})))
return failure();
this->patterns =
@@ -162,11 +160,7 @@ class ConvertToEmitC : public impl::ConvertToEmitCBase<ConvertToEmitC> {
LogicalResult initialize(MLIRContext *context) final {
std::shared_ptr<ConvertToEmitCPassInterface> impl;
- std::optional<bool> lowerToCppOverride;
- if (this->lowerToCpp.hasValue())
- lowerToCppOverride = this->lowerToCpp;
- impl = std::make_shared<StaticConvertToEmitC>(context, filterDialects,
- lowerToCppOverride);
+ impl = std::make_shared<StaticConvertToEmitC>(context, filterDialects);
if (failed(impl->initialize()))
return failure();
this->impl = impl;
@@ -186,10 +180,8 @@ class ConvertToEmitC : public impl::ConvertToEmitCBase<ConvertToEmitC> {
//===----------------------------------------------------------------------===//
ConvertToEmitCPassInterface::ConvertToEmitCPassInterface(
- MLIRContext *context, ArrayRef<std::string> filterDialects,
- std::optional<bool> lowerToCpp)
- : context(context), filterDialects(filterDialects), lowerToCpp(lowerToCpp) {
-}
+ MLIRContext *context, ArrayRef<std::string> filterDialects)
+ : context(context), filterDialects(filterDialects) {}
void ConvertToEmitCPassInterface::getDependentDialects(
DialectRegistry ®istry) {
diff --git a/mlir/lib/Conversion/FuncToEmitC/FuncToEmitC.cpp b/mlir/lib/Conversion/FuncToEmitC/FuncToEmitC.cpp
index 27fd81ba2eca7..4801f07d82c9f 100644
--- a/mlir/lib/Conversion/FuncToEmitC/FuncToEmitC.cpp
+++ b/mlir/lib/Conversion/FuncToEmitC/FuncToEmitC.cpp
@@ -16,122 +16,12 @@
#include "mlir/Conversion/ConvertToEmitC/ToEmitCInterface.h"
#include "mlir/Dialect/EmitC/IR/EmitC.h"
#include "mlir/Dialect/Func/IR/FuncOps.h"
-#include "mlir/IR/BuiltinAttributes.h"
-#include "mlir/IR/SymbolTable.h"
#include "mlir/Transforms/DialectConversion.h"
-#include "llvm/ADT/StringExtras.h"
-#include "llvm/Support/LogicalResult.h"
using namespace mlir;
namespace {
-//===----------------------------------------------------------------------===//
-// Multi-return struct helpers
-//===----------------------------------------------------------------------===//
-
-// Looks up or creates an `emitc.class` named after `types` in the nearest
-// enclosing symbol table of `op`, suitable for packing those types as plain
-// struct fields (field0, field1, ...). If the class already exists it is
-// verified to have exactly the right fields and no methods. Returns the
-// corresponding !emitc.opaque<"struct ..."> type on success.
-static FailureOr<emitc::OpaqueType>
-getOrCreateMultiReturnType(ConversionPatternRewriter &rewriter, Location loc,
- Operation *op, TypeRange types) {
- // Build the struct name from the types, e.g. "return_i32_i32". Each type is
- // printed and non-alphanumeric characters are replaced with '_'.
- std::string structName = "return";
- for (Type type : types) {
- std::string typeName;
- llvm::raw_string_ostream os(typeName);
- type.print(os);
- std::replace_if(
- typeName.begin(), typeName.end(),
- [](char c) { return !llvm::isAlnum(c); }, '_');
- structName += "_" + typeName;
- }
-
- // Find the enclosing symbol table and the direct child op within it that
- // contains `op`; the class will be inserted immediately before that child.
- Operation *symbolTableOp = SymbolTable::getNearestSymbolTable(op);
- Operation *insertBefore = op;
- while (insertBefore->getParentOp() != symbolTableOp)
- insertBefore = insertBefore->getParentOp();
-
- if (Operation *sym = SymbolTable::lookupSymbolIn(symbolTableOp, structName)) {
- auto classOp = dyn_cast<emitc::ClassOp>(sym);
- if (!classOp)
- return emitError(loc) << "symbol '" << structName
- << "' exists but is not an emitc.class";
-
- if (classOp.getClassType() != emitc::ClassType::struct_)
- return emitError(loc)
- << "existing class '" << structName << "' is not a struct";
-
- SmallVector<emitc::FieldOp> fields;
- for (Operation &bodyOp : classOp.getBody().front()) {
- if (isa<emitc::FuncOp>(bodyOp))
- return emitError(loc) << "existing class '" << structName
- << "' has methods; expected a plain struct";
- if (auto fieldOp = dyn_cast<emitc::FieldOp>(bodyOp))
- fields.push_back(fieldOp);
- }
- if (fields.size() != types.size())
- return emitError(loc) << "existing class '" << structName
- << "' has wrong number of fields";
- for (auto [i, fieldOp] : llvm::enumerate(fields)) {
- if (fieldOp.getSymName() != "field" + std::to_string(i))
- return emitError(loc) << "existing class '" << structName
- << "': unexpected field name at index " << i;
- if (fieldOp.getTypeAttr().getValue() != types[i])
- return emitError(loc) << "existing class '" << structName
- << "': wrong type for field " << i;
- }
- } else {
- // Create the ClassOp before `insertBefore`, then restore the insertion
- // point.
- auto savedIP = rewriter.saveInsertionPoint();
- rewriter.setInsertionPoint(insertBefore);
-
- emitc::ClassOp classOp = emitc::ClassOp::create(rewriter, loc, structName,
- /*final_specifier=*/false,
- emitc::ClassType::struct_);
- rewriter.createBlock(&classOp.getBody());
- rewriter.setInsertionPointToStart(&classOp.getBody().front());
-
- for (auto [i, type] : llvm::enumerate(types)) {
- auto fieldName = rewriter.getStringAttr("field" + std::to_string(i));
- emitc::FieldOp::create(rewriter, loc, fieldName, TypeAttr::get(type),
- nullptr);
- }
-
- rewriter.restoreInsertionPoint(savedIP);
- }
- return emitc::OpaqueType::get(rewriter.getContext(), "struct " + structName);
-}
-
-// Packs multiple SSA values into an emitc.class struct variable and loads the
-// result as a single SSA value of the opaque struct type.
-static Value packValuesIntoStruct(ConversionPatternRewriter &rewriter,
- Location loc, ValueRange values,
- emitc::OpaqueType structType) {
- MLIRContext *ctx = rewriter.getContext();
- auto noInit = emitc::OpaqueAttr::get(ctx, "");
- Value structLv =
- emitc::VariableOp::create(rewriter, loc,
- emitc::LValueType::get(structType), noInit)
- .getResult();
- for (auto [i, val] : llvm::enumerate(values)) {
- Value fieldLv =
- emitc::MemberOp::create(
- rewriter, loc, emitc::LValueType::get(val.getType()),
- rewriter.getStringAttr("field" + std::to_string(i)), structLv)
- .getResult();
- emitc::AssignOp::create(rewriter, loc, fieldLv, val);
- }
- return emitc::LoadOp::create(rewriter, loc, structType, structLv).getResult();
-}
-
/// Implement the interface to convert Func to EmitC.
struct FuncToEmitCDialectInterface : public ConvertToEmitCPatternInterface {
FuncToEmitCDialectInterface(Dialect *dialect)
@@ -141,9 +31,8 @@ struct FuncToEmitCDialectInterface : public ConvertToEmitCPatternInterface {
/// and mark dialect legal for the conversion target.
void populateConvertToEmitCConversionPatterns(
ConversionTarget &target, TypeConverter &typeConverter,
- RewritePatternSet &patterns, std::optional<bool> lowerToCpp) const final {
- populateFuncToEmitCPatterns(typeConverter, patterns,
- lowerToCpp.value_or(true));
+ RewritePatternSet &patterns) const final {
+ populateFuncToEmitCPatterns(typeConverter, patterns);
}
};
} // namespace
@@ -161,102 +50,45 @@ void mlir::registerConvertFuncToEmitCInterface(DialectRegistry ®istry) {
namespace {
class CallOpConversion final : public OpConversionPattern<func::CallOp> {
public:
- CallOpConversion(const TypeConverter &typeConverter, MLIRContext *ctx,
- bool lowerToCpp)
- : OpConversionPattern<func::CallOp>(typeConverter, ctx),
- lowerToCpp(lowerToCpp) {}
+ using OpConversionPattern<func::CallOp>::OpConversionPattern;
LogicalResult
matchAndRewrite(func::CallOp callOp, OpAdaptor adaptor,
ConversionPatternRewriter &rewriter) const override {
- // Do not convert multiple-return functions if lowering target is Cpp.
- // The translator will emit the return values as an std::tuple.
- if (callOp.getNumResults() > 1 && lowerToCpp)
+ // Multiple results func cannot be converted to `emitc.func`.
+ if (callOp.getNumResults() > 1)
return rewriter.notifyMatchFailure(
callOp, "only functions with zero or one result can be converted");
- SmallVector<Type> convertedResultTypes;
- for (Type t : callOp.getResultTypes()) {
- Type resultType = getTypeConverter()->convertType(t);
+ if (callOp.getNumResults() == 1) {
+ Type resultType =
+ getTypeConverter()->convertType(callOp.getResult(0).getType());
if (!resultType)
return rewriter.notifyMatchFailure(callOp,
"result type conversion failed");
if (isa<emitc::ArrayType>(resultType))
return rewriter.notifyMatchFailure(
callOp, "function calls returning arrays are not supported");
- convertedResultTypes.push_back(resultType);
- }
-
- if (callOp.getNumResults() <= 1) {
- rewriter.replaceOpWithNewOp<emitc::CallOp>(
- callOp, callOp.getResultTypes(), adaptor.getOperands(),
- callOp->getAttrs());
- return success();
}
- // Multi-result call: determine the struct type.
- Location loc = callOp.getLoc();
-
- auto structType =
- getOrCreateMultiReturnType(rewriter, loc, callOp, convertedResultTypes);
- if (failed(structType))
- return rewriter.notifyMatchFailure(callOp,
- "incompatible multi-return struct");
-
- // Emit a call returning the packed struct.
- Value structVal =
- emitc::CallOp::create(rewriter, loc, callOp.getCalleeAttr(),
- TypeRange{*structType}, adaptor.getOperands())
- .getResult(0);
+ rewriter.replaceOpWithNewOp<emitc::CallOp>(callOp, callOp.getResultTypes(),
+ adaptor.getOperands(),
+ callOp->getAttrs());
- // Unpack struct fields to replace the original multiple results.
- MLIRContext *ctx = rewriter.getContext();
- auto noInit = emitc::OpaqueAttr::get(ctx, "");
- Value structLv =
- emitc::VariableOp::create(rewriter, loc,
- emitc::LValueType::get(*structType), noInit)
- .getResult();
- emitc::AssignOp::create(rewriter, loc, structLv, structVal);
- SmallVector<Value> results;
- for (auto [i, result] : llvm::enumerate(callOp.getResults())) {
- if (result.use_empty()) {
- results.push_back(Value()); // No replacement needed.
- continue;
- }
- Type fieldType = convertedResultTypes[i];
- StringAttr fieldName =
- rewriter.getStringAttr("field" + std::to_string(i));
- Value fieldLv = emitc::MemberOp::create(rewriter, loc,
- emitc::LValueType::get(fieldType),
- fieldName, structLv)
- .getResult();
- results.push_back(
- emitc::LoadOp::create(rewriter, loc, fieldType, fieldLv).getResult());
- }
-
- rewriter.replaceOp(callOp, results);
return success();
}
-
-private:
- bool lowerToCpp;
};
class FuncOpConversion final : public OpConversionPattern<func::FuncOp> {
public:
- FuncOpConversion(const TypeConverter &typeConverter, MLIRContext *ctx,
- bool lowerToCpp)
- : OpConversionPattern<func::FuncOp>(typeConverter, ctx),
- lowerToCpp(lowerToCpp) {}
+ using OpConversionPattern<func::FuncOp>::OpConversionPattern;
LogicalResult
matchAndRewrite(func::FuncOp funcOp, OpAdaptor adaptor,
ConversionPatternRewriter &rewriter) const override {
FunctionType fnType = funcOp.getFunctionType();
- // Do not convert multiple-return functions if lowering target is Cpp.
- // The translator will emit the return values as an std::tuple.
- if (fnType.getNumResults() > 1 && lowerToCpp)
+ if (fnType.getNumResults() > 1)
return rewriter.notifyMatchFailure(
funcOp, "only functions with zero or one result can be converted");
@@ -270,28 +102,15 @@ class FuncOpConversion final : public OpConversionPattern<func::FuncOp> {
signatureConverter.addInputs(argType.index(), convertedType);
}
- SmallVector<Type> convertedResultTypes;
- for (Type t : fnType.getResults()) {
- Type resultType = getTypeConverter()->convertType(t);
+ Type resultType;
+ if (fnType.getNumResults() == 1) {
+ resultType = getTypeConverter()->convertType(fnType.getResult(0));
if (!resultType)
return rewriter.notifyMatchFailure(funcOp,
"result type conversion failed");
if (isa<emitc::ArrayType>(resultType))
return rewriter.notifyMatchFailure(
funcOp, "functions returning arrays are not supported");
- convertedResultTypes.push_back(resultType);
- }
-
- Type resultType;
- if (fnType.getNumResults() == 1) {
- resultType = convertedResultTypes[0];
- } else if (fnType.getNumResults() > 1) {
- auto structTypeOrErr = getOrCreateMultiReturnType(
- rewriter, funcOp.getLoc(), funcOp, convertedResultTypes);
- if (failed(structTypeOrErr))
- return rewriter.notifyMatchFailure(funcOp,
- "incompatible multi-return struct");
- resultType = *structTypeOrErr;
}
// Create the converted `emitc.func` op.
@@ -332,57 +151,28 @@ class FuncOpConversion final : public OpConversionPattern<func::FuncOp> {
return success();
}
-
-private:
- bool lowerToCpp;
};
class ReturnOpConversion final : public OpConversionPattern<func::ReturnOp> {
public:
- ReturnOpConversion(const TypeConverter &typeConverter, MLIRContext *ctx,
- bool lowerToCpp)
- : OpConversionPattern<func::ReturnOp>(typeConverter, ctx),
- lowerToCpp(lowerToCpp) {}
+ using OpConversionPattern<func::ReturnOp>::OpConversionPattern;
LogicalResult
matchAndRewrite(func::ReturnOp returnOp, OpAdaptor adaptor,
ConversionPatternRewriter &rewriter) const override {
- // Do not convert multiple-return functions if lowering target is Cpp.
- // The translator will emit the return values as an std::tuple.
- if (returnOp.getNumOperands() > 1 && lowerToCpp)
+ if (returnOp.getNumOperands() > 1)
return rewriter.notifyMatchFailure(
returnOp, "only zero or one operan...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/202911
More information about the Mlir-commits
mailing list