[Mlir-commits] [mlir] 168213f - [mlir] Move data layout from LLVMDialect to module Op attributes
Alex Zinenko
llvmlistbot at llvm.org
Mon Aug 17 06:12:43 PDT 2020
Author: Alex Zinenko
Date: 2020-08-17T15:12:36+02:00
New Revision: 168213f91c571352c56f432573513cba3f9ba61b
URL: https://github.com/llvm/llvm-project/commit/168213f91c571352c56f432573513cba3f9ba61b
DIFF: https://github.com/llvm/llvm-project/commit/168213f91c571352c56f432573513cba3f9ba61b.diff
LOG: [mlir] Move data layout from LLVMDialect to module Op attributes
Legacy implementation of the LLVM dialect in MLIR contained an instance of
llvm::Module as it was required to parse LLVM IR types. The access to the data
layout of this module was exposed to the users for convenience, but in practice
this layout has always been the default one obtained by parsing an empty layout
description string. Current implementation of the dialect no longer relies on
wrapping LLVM IR types, but it kept an instance of DataLayout for
compatibility. This effectively forces a single data layout to be used across
all modules in a given MLIR context, which is not desirable. Remove DataLayout
from the LLVM dialect and attach it as a module attribute instead. Since MLIR
does not yet have support for data layouts, use the LLVM DataLayout in string
form with verification inside MLIR. Introduce the layout when converting a
module to the LLVM dialect and keep the default "" description for
compatibility.
This approach should be replaced with a proper MLIR-based data layout when it
becomes available, but provides an immediate solution to compiling modules with
different layouts, e.g. for GPUs.
This removes the need for LLVMDialectImpl, which is also removed.
Depends On D85650
Reviewed By: aartbik
Differential Revision: https://reviews.llvm.org/D85652
Added:
mlir/test/Conversion/StandardToLLVM/convert-data-layout.mlir
Modified:
mlir/include/mlir/Conversion/Passes.td
mlir/include/mlir/Conversion/StandardToLLVM/ConvertStandardToLLVM.h
mlir/include/mlir/Conversion/StandardToLLVM/ConvertStandardToLLVMPass.h
mlir/include/mlir/Dialect/LLVMIR/LLVMOpBase.td
mlir/lib/Conversion/StandardToLLVM/StandardToLLVM.cpp
mlir/lib/Conversion/VectorToLLVM/ConvertVectorToLLVM.cpp
mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
mlir/test/Dialect/LLVMIR/invalid.mlir
mlir/test/Target/llvmir.mlir
Removed:
################################################################################
diff --git a/mlir/include/mlir/Conversion/Passes.td b/mlir/include/mlir/Conversion/Passes.td
index 4d4fe064a6bc..4ff23d71a5c0 100644
--- a/mlir/include/mlir/Conversion/Passes.td
+++ b/mlir/include/mlir/Conversion/Passes.td
@@ -277,6 +277,10 @@ def ConvertStandardToLLVM : Pass<"convert-std-to-llvm", "ModuleOp"> {
Option<"indexBitwidth", "index-bitwidth", "unsigned",
/*default=kDeriveIndexBitwidthFromDataLayout*/"0",
"Bitwidth of the index type, 0 to use size of machine word">,
+ Option<"dataLayout", "data-layout", "std::string",
+ /*default=*/"\"\"",
+ "String description (LLVM format) of the data layout that is "
+ "expected on the produced module">
];
}
diff --git a/mlir/include/mlir/Conversion/StandardToLLVM/ConvertStandardToLLVM.h b/mlir/include/mlir/Conversion/StandardToLLVM/ConvertStandardToLLVM.h
index 2f8f87fa0e41..63ffd7837382 100644
--- a/mlir/include/mlir/Conversion/StandardToLLVM/ConvertStandardToLLVM.h
+++ b/mlir/include/mlir/Conversion/StandardToLLVM/ConvertStandardToLLVM.h
@@ -105,6 +105,9 @@ class LLVMTypeConverter : public TypeConverter {
/// pointers to memref descriptors for arguments.
LLVM::LLVMType convertFunctionTypeCWrapper(FunctionType type);
+ /// Returns the data layout to use during and after conversion.
+ const llvm::DataLayout &getDataLayout() { return options.dataLayout; }
+
/// Gets the LLVM representation of the index type. The returned type is an
/// integer type with the size configured for this type converter.
LLVM::LLVMType getIndexType();
diff --git a/mlir/include/mlir/Conversion/StandardToLLVM/ConvertStandardToLLVMPass.h b/mlir/include/mlir/Conversion/StandardToLLVM/ConvertStandardToLLVMPass.h
index 3d8312c6e7f5..02fefc689bac 100644
--- a/mlir/include/mlir/Conversion/StandardToLLVM/ConvertStandardToLLVMPass.h
+++ b/mlir/include/mlir/Conversion/StandardToLLVM/ConvertStandardToLLVMPass.h
@@ -9,6 +9,8 @@
#ifndef MLIR_CONVERSION_STANDARDTOLLVM_CONVERTSTANDARDTOLLVMPASS_H_
#define MLIR_CONVERSION_STANDARDTOLLVM_CONVERTSTANDARDTOLLVMPASS_H_
+#include "llvm/IR/DataLayout.h"
+
#include <memory>
namespace mlir {
@@ -31,6 +33,11 @@ struct LowerToLLVMOptions {
/// Use aligned_alloc for heap allocations.
bool useAlignedAlloc = false;
+ /// The data layout of the module to produce. This must be consistent with the
+ /// data layout used in the upper levels of the lowering pipeline.
+ // TODO: this should be replaced by MLIR data layout when one exists.
+ llvm::DataLayout dataLayout = llvm::DataLayout("");
+
/// Get a statically allocated copy of the default LowerToLLVMOptions.
static const LowerToLLVMOptions &getDefaultOptions() {
static LowerToLLVMOptions options;
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMOpBase.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMOpBase.td
index d21f5bc0b49b..e824f97bc285 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMOpBase.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMOpBase.td
@@ -20,16 +20,15 @@ def LLVM_Dialect : Dialect {
let name = "llvm";
let cppNamespace = "LLVM";
let hasRegionArgAttrVerify = 1;
+ let hasOperationAttrVerify = 1;
let extraClassDeclaration = [{
- ~LLVMDialect();
- const llvm::DataLayout &getDataLayout();
+ /// Name of the data layout attributes.
+ static StringRef getDataLayoutAttrName() { return "llvm.data_layout"; }
- private:
- friend LLVMType;
-
- // This can't be a unique_ptr because the ctor is generated inline
- // in the class definition at the moment.
- detail::LLVMDialectImpl *impl;
+ /// Verifies if the given string is a well-formed data layout descriptor.
+ /// Uses `reportError` to report errors.
+ static LogicalResult verifyDataLayoutString(
+ StringRef descr, llvm::function_ref<void (const Twine &)> reportError);
}];
}
diff --git a/mlir/lib/Conversion/StandardToLLVM/StandardToLLVM.cpp b/mlir/lib/Conversion/StandardToLLVM/StandardToLLVM.cpp
index efe4a3c958d7..4a061963fce3 100644
--- a/mlir/lib/Conversion/StandardToLLVM/StandardToLLVM.cpp
+++ b/mlir/lib/Conversion/StandardToLLVM/StandardToLLVM.cpp
@@ -129,8 +129,7 @@ LLVMTypeConverter::LLVMTypeConverter(MLIRContext *ctx,
options(options) {
assert(llvmDialect && "LLVM IR dialect is not registered");
if (options.indexBitwidth == kDeriveIndexBitwidthFromDataLayout)
- this->options.indexBitwidth =
- llvmDialect->getDataLayout().getPointerSizeInBits();
+ this->options.indexBitwidth = options.dataLayout.getPointerSizeInBits();
// Register conversions for the standard types.
addConversion([&](ComplexType type) { return convertComplexType(type); });
@@ -198,7 +197,7 @@ LLVM::LLVMType LLVMTypeConverter::getIndexType() {
}
unsigned LLVMTypeConverter::getPointerBitwidth(unsigned addressSpace) {
- return llvmDialect->getDataLayout().getPointerSizeInBits(addressSpace);
+ return options.dataLayout.getPointerSizeInBits(addressSpace);
}
Type LLVMTypeConverter::convertIndexType(IndexType type) {
@@ -3427,11 +3426,13 @@ namespace {
struct LLVMLoweringPass : public ConvertStandardToLLVMBase<LLVMLoweringPass> {
LLVMLoweringPass() = default;
LLVMLoweringPass(bool useBarePtrCallConv, bool emitCWrappers,
- unsigned indexBitwidth, bool useAlignedAlloc) {
+ unsigned indexBitwidth, bool useAlignedAlloc,
+ const llvm::DataLayout &dataLayout) {
this->useBarePtrCallConv = useBarePtrCallConv;
this->emitCWrappers = emitCWrappers;
this->indexBitwidth = indexBitwidth;
this->useAlignedAlloc = useAlignedAlloc;
+ this->dataLayout = dataLayout.getStringRepresentation();
}
/// Run the dialect converter on the module.
@@ -3443,11 +3444,19 @@ struct LLVMLoweringPass : public ConvertStandardToLLVMBase<LLVMLoweringPass> {
signalPassFailure();
return;
}
+ if (failed(LLVM::LLVMDialect::verifyDataLayoutString(
+ this->dataLayout, [this](const Twine &message) {
+ getOperation().emitError() << message.str();
+ }))) {
+ signalPassFailure();
+ return;
+ }
ModuleOp m = getOperation();
LowerToLLVMOptions options = {useBarePtrCallConv, emitCWrappers,
- indexBitwidth, useAlignedAlloc};
+ indexBitwidth, useAlignedAlloc,
+ llvm::DataLayout(this->dataLayout)};
LLVMTypeConverter typeConverter(&getContext(), options);
OwningRewritePatternList patterns;
@@ -3456,6 +3465,8 @@ struct LLVMLoweringPass : public ConvertStandardToLLVMBase<LLVMLoweringPass> {
LLVMConversionTarget target(getContext());
if (failed(applyPartialConversion(m, target, patterns)))
signalPassFailure();
+ m.setAttr(LLVM::LLVMDialect::getDataLayoutAttrName(),
+ StringAttr::get(this->dataLayout, m.getContext()));
}
};
} // end namespace
@@ -3471,5 +3482,5 @@ std::unique_ptr<OperationPass<ModuleOp>>
mlir::createLowerToLLVMPass(const LowerToLLVMOptions &options) {
return std::make_unique<LLVMLoweringPass>(
options.useBarePtrCallConv, options.emitCWrappers, options.indexBitwidth,
- options.useAlignedAlloc);
+ options.useAlignedAlloc, options.dataLayout);
}
diff --git a/mlir/lib/Conversion/VectorToLLVM/ConvertVectorToLLVM.cpp b/mlir/lib/Conversion/VectorToLLVM/ConvertVectorToLLVM.cpp
index a68f28fa1289..45f6f4d1cf31 100644
--- a/mlir/lib/Conversion/VectorToLLVM/ConvertVectorToLLVM.cpp
+++ b/mlir/lib/Conversion/VectorToLLVM/ConvertVectorToLLVM.cpp
@@ -128,11 +128,10 @@ LogicalResult getMemRefAlignment(LLVMTypeConverter &typeConverter, T op,
// TODO: this should use the MLIR data layout when it becomes available and
// stop depending on translation.
- LLVM::LLVMDialect *dialect = typeConverter.getDialect();
llvm::LLVMContext llvmContext;
align = LLVM::TypeToLLVMIRTranslator(llvmContext)
.getPreferredAlignment(elementTy.cast<LLVM::LLVMType>(),
- dialect->getDataLayout());
+ typeConverter.getDataLayout());
return success();
}
diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
index f1299c065d36..6c5f207b57a6 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
@@ -1668,23 +1668,7 @@ static LogicalResult verify(FenceOp &op) {
// LLVMDialect initialization, type parsing, and registration.
//===----------------------------------------------------------------------===//
-namespace mlir {
-namespace LLVM {
-namespace detail {
-struct LLVMDialectImpl {
- LLVMDialectImpl() : layout("") {}
-
- /// Default data layout to use.
- // TODO: this should be moved to some Op equivalent to LLVM module and
- // eventually replaced with a proper MLIR data layout.
- llvm::DataLayout layout;
-};
-} // end namespace detail
-} // end namespace LLVM
-} // end namespace mlir
-
void LLVMDialect::initialize() {
- impl = new detail::LLVMDialectImpl();
// clang-format off
addTypes<LLVMVoidType,
LLVMHalfType,
@@ -1715,13 +1699,9 @@ void LLVMDialect::initialize() {
allowUnknownOperations();
}
-LLVMDialect::~LLVMDialect() { delete impl; }
-
#define GET_OP_CLASSES
#include "mlir/Dialect/LLVMIR/LLVMOps.cpp.inc"
-const llvm::DataLayout &LLVMDialect::getDataLayout() { return impl->layout; }
-
/// Parse a type registered to this dialect.
Type LLVMDialect::parseType(DialectAsmParser &parser) const {
return detail::parseType(parser);
@@ -1732,6 +1712,39 @@ void LLVMDialect::printType(Type type, DialectAsmPrinter &os) const {
return detail::printType(type.cast<LLVMType>(), os);
}
+LogicalResult LLVMDialect::verifyDataLayoutString(
+ StringRef descr, llvm::function_ref<void(const Twine &)> reportError) {
+ llvm::Expected<llvm::DataLayout> maybeDataLayout =
+ llvm::DataLayout::parse(descr);
+ if (maybeDataLayout)
+ return success();
+
+ std::string message;
+ llvm::raw_string_ostream messageStream(message);
+ llvm::logAllUnhandledErrors(maybeDataLayout.takeError(), messageStream);
+ reportError("invalid data layout descriptor: " + messageStream.str());
+ return failure();
+}
+
+/// Verify LLVM dialect attributes.
+LogicalResult LLVMDialect::verifyOperationAttribute(Operation *op,
+ NamedAttribute attr) {
+ // If the data layout attribute is present, it must use the LLVM data layout
+ // syntax. Try parsing it and report errors in case of failure. Users of this
+ // attribute may assume it is well-formed and can pass it to the (asserting)
+ // llvm::DataLayout constructor.
+ if (attr.first.strref() != LLVM::LLVMDialect::getDataLayoutAttrName())
+ return success();
+ if (auto stringAttr = attr.second.dyn_cast<StringAttr>())
+ return verifyDataLayoutString(
+ stringAttr.getValue(),
+ [op](const Twine &message) { op->emitOpError() << message.str(); });
+
+ return op->emitOpError() << "expected '"
+ << LLVM::LLVMDialect::getDataLayoutAttrName()
+ << "' to be a string attribute";
+}
+
/// Verify LLVMIR function argument attributes.
LogicalResult LLVMDialect::verifyRegionArgAttribute(Operation *op,
unsigned regionIdx,
diff --git a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
index 215c1910f744..f8277d154f27 100644
--- a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
@@ -944,11 +944,11 @@ ModuleTranslation::lookupValues(ValueRange values) {
std::unique_ptr<llvm::Module> ModuleTranslation::prepareLLVMModule(
Operation *m, llvm::LLVMContext &llvmContext, StringRef name) {
- auto *dialect = m->getContext()->getRegisteredDialect<LLVM::LLVMDialect>();
- assert(dialect && "LLVM dialect must be registered");
-
auto llvmModule = std::make_unique<llvm::Module>(name, llvmContext);
- llvmModule->setDataLayout(dialect->getDataLayout());
+
+ if (auto dataLayoutAttr =
+ m->getAttr(LLVM::LLVMDialect::getDataLayoutAttrName()))
+ llvmModule->setDataLayout(dataLayoutAttr.cast<StringAttr>().getValue());
// Inject declarations for `malloc` and `free` functions that can be used in
// memref allocation/deallocation coming from standard ops lowering.
diff --git a/mlir/test/Conversion/StandardToLLVM/convert-data-layout.mlir b/mlir/test/Conversion/StandardToLLVM/convert-data-layout.mlir
new file mode 100644
index 000000000000..5086de2f5d05
--- /dev/null
+++ b/mlir/test/Conversion/StandardToLLVM/convert-data-layout.mlir
@@ -0,0 +1,6 @@
+// RUN: mlir-opt -convert-std-to-llvm %s | FileCheck %s
+// RUN-32: mlir-opt -convert-std-to-llvm='data-layout=p:32:32:32' %s | FileCheck %s
+
+// CHECK: module attributes {llvm.data_layout = ""}
+// CHECK-32: module attributes {llvm.data_layout ="p:32:32:32"}
+module {}
diff --git a/mlir/test/Dialect/LLVMIR/invalid.mlir b/mlir/test/Dialect/LLVMIR/invalid.mlir
index b4475df66fd1..737fa4ff8bf1 100644
--- a/mlir/test/Dialect/LLVMIR/invalid.mlir
+++ b/mlir/test/Dialect/LLVMIR/invalid.mlir
@@ -603,3 +603,10 @@ func @invalid_ordering_in_fence() {
// expected-error @+1 {{can be given only acquire, release, acq_rel, and seq_cst orderings}}
llvm.fence syncscope("agent") monotonic
}
+
+// -----
+
+// expected-error @+1 {{invalid data layout descriptor}}
+module attributes {llvm.data_layout = "#vjkr32"} {
+ func @invalid_data_layout()
+}
diff --git a/mlir/test/Target/llvmir.mlir b/mlir/test/Target/llvmir.mlir
index 5e57f1c7c698..b1abae64a2ad 100644
--- a/mlir/test/Target/llvmir.mlir
+++ b/mlir/test/Target/llvmir.mlir
@@ -1295,3 +1295,19 @@ llvm.func @nontemoral_store_and_load() {
}
// CHECK: ![[NODE]] = !{i32 1}
+
+// -----
+
+// Check that the translation does not crash in absence of a data layout.
+module {
+ // CHECK: declare void @module_default_layout
+ llvm.func @module_default_layout()
+}
+
+// -----
+
+// CHECK: target datalayout = "E"
+module attributes {llvm.data_layout = "E"} {
+ llvm.func @module_big_endian()
+}
+
More information about the Mlir-commits
mailing list