-
Notifications
You must be signed in to change notification settings - Fork 14.4k
[MLIR][LLVMIR][DLTI] Add #llvm.target, #llvm.data_layout and TargetAttrInterface #145899
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
base: main
Are you sure you want to change the base?
Conversation
✅ With the latest revision this PR passed the C/C++ code formatter. |
return failure(); | ||
} | ||
|
||
targetMachine = {target->createTargetMachine( |
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.
@rengolin, what's the ownership model of TargetMachine
s? Should this attribute become the owner of this TargetMachine *
by using a unique_ptr
?
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.
TargetMachine
can be put into a unique_ptr
, then reset
using an llvm::Target::createTargetMachine
and then use .get()
to pass it to the LLVM pipeline builder.
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.
I remembered why I didn't do it before: wrapping TargetMachine
in a unique_ptr makes ODS-generated code, in particular a printer, scream:
In file included from /data/nfs_home/rmorel/llvm-project.git/llvm-target-dlti/mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp:20:
/data/nfs_home/rmorel/llvm-project.git/llvm-target-dlti/llvm/include/llvm/ADT/TypeSwitch.h:102:29: error: call to implicitly-deleted copy constructor of 'mlir::LLVM::TargetAttr'
result.emplace(caseFn(caseValue));
^~~~~~~~~
tools/mlir/include/mlir/Dialect/LLVMIR/LLVMOpsAttrDefs.cpp.inc:372:80: note: in instantiation of function template specialization 'llvm::TypeSwitch<mlir::Attribute, llvm::LogicalResult>::Case<mlir::LLVM::TargetAttr, (lambda at tools/mlir/include/mlir/Dialect/LLVMIR/LLVMOpsAttrDefs.cpp.inc:372:111)>' requested here
return ::llvm::TypeSwitch<::mlir::Attribute, ::llvm::LogicalResult>(def) .Case<::mlir::LLVM::TargetAttr>([&](auto t) {
^
tools/mlir/include/mlir/Dialect/LLVMIR/LLVMOpsAttrDefs.h.inc:306:55: note: copy constructor of 'TargetAttr' is implicitly deleted because field 'targetMachine' has a deleted copy constructor
std::optional<std::unique_ptr<llvm::TargetMachine>> targetMachine = std::nullopt;
^
/opt/rh/gcc-toolset-13/root/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/optional:707:7: note: copy constructor of 'optional<std::unique_ptr<llvm::TargetMachine>>' is implicitly deleted because base class '_Enable_copy_move<is_copy_constructible_v<unique_ptr<TargetMachine, default_delete<TargetMachine>>>, __and_v<is_copy_constructible<unique_ptr<TargetMachine, default_delete<TargetMachine>>>, is_copy_assignable<unique_ptr<TargetMachine, default_delete<TargetMachine>>>>, is_move_constructible_v<unique_ptr<TargetMachine, default_delete<TargetMachine>>>, __and_v<is_move_constructible<unique_ptr<TargetMachine, default_delete<TargetMachine>>>, is_move_assignable<unique_ptr<TargetMachine, default_delete<TargetMachine>>>>, optional<unique_ptr<TargetMachine, default_delete<TargetMachine>>>>' has a deleted copy constructor
private _Enable_copy_move<
^
/opt/rh/gcc-toolset-13/root/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/enable_special_members.h:160:15: note: '_Enable_copy_move' has been explicitly marked deleted here
constexpr _Enable_copy_move(_Enable_copy_move const&) noexcept = delete;
^
tools/mlir/include/mlir/Dialect/LLVMIR/LLVMOpsAttrDefs.cpp.inc:372:120: note: passing argument to parameter 't' here
return ::llvm::TypeSwitch<::mlir::Attribute, ::llvm::LogicalResult>(def) .Case<::mlir::LLVM::TargetAttr>([&](auto t) {
^
1 error generated.
This is with std::optional<std::unique_ptr<llvm::TargetMachine>> targetMachine = std::nullopt;
in the extraClassDeclaration
and
auto targetMach = std::unique_ptr<llvm::TargetMachine>(target->createTargetMachine(
llvm::Triple(getTriple().strref()), getChip().strref(),
getFeatures().getFeaturesString().c_str(), {}, {}));
targetMachine = { std::move(targetMach) };
in the implementation.
If I am understanding correctly, the issue seems to be that the type switch in the following code is trying to do a copy of the attribute, even though attributes should just be thought of/treated as (unique) pointers (... if my understanding is correct):
static ::llvm::LogicalResult generatedAttributePrinter(::mlir::Attribute def, ::mlir::AsmPrinter &printer) {
return ::llvm::TypeSwitch<::mlir::Attribute, ::llvm::LogicalResult>(def) .Case<::mlir::LLVM::TargetAttr>([&](auto t) {
printer << ::mlir::LLVM::TargetAttr::getMnemonic();
t.print(printer);
return ::mlir::success();
})
.Case<::mlir::LLVM::DataLayoutAttr>([&](auto t) {
printer << ::mlir::LLVM::DataLayoutAttr::getMnemonic();
t.print(printer);
return ::mlir::success();
})
.Case<::mlir::LLVM::CConvAttr>([&](auto t)
...
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.
Hopefully someone could help explain what the right way is to hang owned pointers off of an attribute.
Any and all pointers welcome!
// LLVM_TargetAttr | ||
//===----------------------------------------------------------------------===// | ||
|
||
def LLVM_TargetAttr : LLVM_Attr<"Target", "target", [LLVM_TargetAttrInterface]> { |
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.
@rengolin, I am not sure what we want to do for verification here. Do we want to insist on the specified triple
, cpu
, and target_features
being valid together. And do we verify that through constructing the relevant TargetMachine
?
How would that work for dealing with IR which is annotated with a #llvm.target
specifying a target that your current llvm-project
isn't built with. Is it right that verification fails in that case? (Maybe it is, but it feels wrong as now verification succeeding or failing becomes a function of whether you built MLIR with the right flags.)
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.
One problem we faced on our side earlier was that it's not that easy to know you're constructing an invalid TargetMachine
(link). But validating all possible combinations is just not viable. For downstream targets, you'd have to overload some verification to match their alternatives.
Other tools don't verify much either (other than completely crash if the strings are invalid), so I guess we can start with that. The discussion about which combinations are valid (link) are generally higher in the pipeline (front-end). At the IR level, we can only treat them as "gospel" and crash if we really can't build a TargetMachine
.
680cc11
to
91560a5
Compare
Couple of pain points in need of suggestions/feedback:
|
@llvm/pr-subscribers-mlir-dlti @llvm/pr-subscribers-mlir Author: Rolf Morel (rolfmorel) ChangesPatch is 38.47 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/145899.diff 22 Files Affected:
diff --git a/flang/include/flang/Optimizer/Dialect/FIRType.h b/flang/include/flang/Optimizer/Dialect/FIRType.h
index ecab12de55d61..83077aef8d08d 100644
--- a/flang/include/flang/Optimizer/Dialect/FIRType.h
+++ b/flang/include/flang/Optimizer/Dialect/FIRType.h
@@ -13,6 +13,7 @@
#ifndef FORTRAN_OPTIMIZER_DIALECT_FIRTYPE_H
#define FORTRAN_OPTIMIZER_DIALECT_FIRTYPE_H
+#include "mlir/Dialect/LLVMIR/LLVMTypes.h"
#include "mlir/IR/BuiltinAttributes.h"
#include "mlir/IR/BuiltinTypes.h"
#include "mlir/Interfaces/DataLayoutInterfaces.h"
diff --git a/mlir/include/mlir/Dialect/LLVMIR/CMakeLists.txt b/mlir/include/mlir/Dialect/LLVMIR/CMakeLists.txt
index cfad07e57021f..f1385cdff62be 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/CMakeLists.txt
+++ b/mlir/include/mlir/Dialect/LLVMIR/CMakeLists.txt
@@ -7,12 +7,26 @@ mlir_tablegen(LLVMOpsDialect.h.inc -gen-dialect-decls)
mlir_tablegen(LLVMOpsDialect.cpp.inc -gen-dialect-defs)
mlir_tablegen(LLVMOpsEnums.h.inc -gen-enum-decls)
mlir_tablegen(LLVMOpsEnums.cpp.inc -gen-enum-defs)
-mlir_tablegen(LLVMOpsAttrDefs.h.inc -gen-attrdef-decls
- -attrdefs-dialect=llvm)
+#For LLVMOpsAttrDefs.h.inc, see below.
mlir_tablegen(LLVMOpsAttrDefs.cpp.inc -gen-attrdef-defs
-attrdefs-dialect=llvm)
add_public_tablegen_target(MLIRLLVMOpsIncGen)
+# NB: Separate out LLVMOpsAttrDefs.h.inc generation as generating it
+# through LLVMOps.td ends up defining LLVMTargetFeaturesAttr even
+# though LLVMTargetFeaturesAttrDefs.* is responsible for that.
+set(LLVM_TARGET_DEFINITIONS LLVMAttrAndEnumDefs.td)
+mlir_tablegen(LLVMOpsAttrDefs.h.inc -gen-attrdef-decls -attrdefs-dialect=llvm)
+add_public_tablegen_target(MLIRLLVMAttrsIncGen)
+
+# NB: LLVMTargetFeaturesAttr is split out into its own file
+# to break a recursive dependency: LLVMInterfaces depends
+# on it, and other LLVMAttrs depending on LLVMInterfaces.
+set(LLVM_TARGET_DEFINITIONS LLVMTargetFeaturesAttrDefs.td)
+mlir_tablegen(LLVMTargetFeaturesAttrDefs.h.inc -gen-attrdef-decls)
+mlir_tablegen(LLVMTargetFeaturesAttrDefs.cpp.inc -gen-attrdef-defs)
+add_public_tablegen_target(MLIRLLVMTargetFeaturesAttrsIncGen)
+
set(LLVM_TARGET_DEFINITIONS LLVMTypes.td)
mlir_tablegen(LLVMTypes.h.inc -gen-typedef-decls -typedefs-dialect=llvm)
mlir_tablegen(LLVMTypes.cpp.inc -gen-typedef-defs -typedefs-dialect=llvm)
diff --git a/mlir/lib/Target/LLVMIR/DataLayoutImporter.h b/mlir/include/mlir/Dialect/LLVMIR/DataLayoutImporter.h
similarity index 82%
rename from mlir/lib/Target/LLVMIR/DataLayoutImporter.h
rename to mlir/include/mlir/Dialect/LLVMIR/DataLayoutImporter.h
index 88ceaf1a74e62..0f036f1c43492 100644
--- a/mlir/lib/Target/LLVMIR/DataLayoutImporter.h
+++ b/mlir/include/mlir/Dialect/LLVMIR/DataLayoutImporter.h
@@ -11,13 +11,14 @@
//
//===----------------------------------------------------------------------===//
-#ifndef MLIR_LIB_TARGET_LLVMIR_DATALAYOUTIMPORTER_H_
-#define MLIR_LIB_TARGET_LLVMIR_DATALAYOUTIMPORTER_H_
+#ifndef MLIR_LLVMIR_DATALAYOUTIMPORTER_H_
+#define MLIR_LLVMIR_DATALAYOUTIMPORTER_H_
#include "mlir/Dialect/LLVMIR/LLVMTypes.h"
#include "mlir/IR/BuiltinAttributes.h"
#include "mlir/Interfaces/DataLayoutInterfaces.h"
#include "llvm/ADT/MapVector.h"
+#include "llvm/IR/DataLayout.h"
namespace llvm {
class StringRef;
@@ -38,6 +39,8 @@ namespace detail {
/// null if the bit width is not supported.
FloatType getFloatType(MLIRContext *context, unsigned width);
+} // namespace detail
+
/// Helper class that translates an LLVM data layout to an MLIR data layout
/// specification. Only integer, float, pointer, alloca memory space, stack
/// alignment, and endianness entries are translated. The class also returns all
@@ -49,7 +52,21 @@ class DataLayoutImporter {
DataLayoutImporter(MLIRContext *context,
const llvm::DataLayout &llvmDataLayout)
: context(context) {
- translateDataLayout(llvmDataLayout);
+ // Transform the data layout to its string representation and append the
+ // default data layout string specified in the language reference
+ // (https://llvm.org/docs/LangRef.html#data-layout). The translation then
+ // parses the string and ignores the default value if a specific kind occurs
+ // in both strings. Additionally, the following default values exist:
+ // - non-default address space pointer specifications default to the default
+ // address space pointer specification
+ // - the alloca address space defaults to the default address space.
+ layoutStr = llvmDataLayout.getStringRepresentation();
+ translateDataLayoutFromStr();
+ }
+
+ DataLayoutImporter(MLIRContext *context, StringRef dataLayoutStr)
+ : layoutStr(dataLayoutStr), context(context) {
+ translateDataLayoutFromStr();
}
/// Returns the MLIR data layout specification translated from the LLVM
@@ -66,7 +83,7 @@ class DataLayoutImporter {
private:
/// Translates the LLVM `dataLayout` to an MLIR data layout specification.
- void translateDataLayout(const llvm::DataLayout &llvmDataLayout);
+ void translateDataLayoutFromStr();
/// Tries to parse the letter only prefix that identifies the specification
/// and removes the consumed characters from the beginning of the string.
@@ -125,8 +142,7 @@ class DataLayoutImporter {
DataLayoutSpecInterface dataLayout;
};
-} // namespace detail
} // namespace LLVM
} // namespace mlir
-#endif // MLIR_LIB_TARGET_LLVMIR_DATALAYOUTIMPORTER_H_
+#endif // MLIR_LLVMIR_DATALAYOUTIMPORTER_H_
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrAndEnumDefs.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrAndEnumDefs.td
new file mode 100644
index 0000000000000..e34375076ffd1
--- /dev/null
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrAndEnumDefs.td
@@ -0,0 +1,10 @@
+//===-- LLVMAttrDefs.td - Solely LLVM Attribute and Enum definitions ----*-===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+include "mlir/Dialect/LLVMIR/LLVMAttrDefs.td"
+include "mlir/Dialect/LLVMIR/LLVMEnums.td"
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
index 790d2e77ea874..e563441d0102b 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
@@ -13,13 +13,138 @@ include "mlir/Dialect/LLVMIR/LLVMDialect.td"
include "mlir/Dialect/LLVMIR/LLVMInterfaces.td"
include "mlir/IR/AttrTypeBase.td"
include "mlir/IR/CommonAttrConstraints.td"
+include "mlir/Interfaces/DataLayoutInterfaces.td"
-// All of the attributes will extend this class.
-class LLVM_Attr<string name, string attrMnemonic,
- list<Trait> traits = [],
- string baseCppClass = "::mlir::Attribute">
- : AttrDef<LLVM_Dialect, name, traits, baseCppClass> {
- let mnemonic = attrMnemonic;
+//===----------------------------------------------------------------------===//
+// LLVM_TargetAttr
+//===----------------------------------------------------------------------===//
+
+def LLVM_TargetAttr : LLVM_Attr<"Target", "target",
+ [LLVM_TargetAttrInterface]> {
+ let summary = "LLVM target info: triple, chip, features";
+ let description = [{
+ An attribute to hold LLVM target information, specifying LLVM's target
+ `triple` string, the target `chip` string (i.e. the `cpu` string), and
+ target `features` string as an attribute. The latter two are optional.
+
+ Has facilities to obtain the corresponding `llvm::TargetMachine` and
+ `llvm::DataLayout`, given the relevant LLVM backend is loaded.
+
+ ---
+
+ Responds to DLTI-queries on the keys:
+ * A query for `"triple"` returns the `StringAttr` for the `triple`.
+ * A query for `"chip"` returns the `StringAttr` for the `chip`/`cpu`, if provided.
+ * A query for `"features"` returns the `TargetFeaturesAttr`, if provided.
+ * Individual features can be queried for on this attribute.
+ }];
+ let parameters = (ins "StringAttr":$triple,
+ OptionalParameter<"StringAttr">:$chip,
+ OptionalParameter<"TargetFeaturesAttr">:$features);
+
+ let assemblyFormat = [{`<` `triple` `=` $triple
+ (`,` `chip` `=` $chip^)?
+ (`,` qualified($features)^)? `>`}];
+
+ let extraClassDeclaration = [{
+ std::optional<llvm::TargetMachine *> targetMachine = std::nullopt;
+
+ FailureOr<llvm::TargetMachine *> getTargetMachine();
+
+ std::optional<llvm::DataLayout> dataLayout = std::nullopt;
+ FailureOr<llvm::DataLayout> getDataLayout();
+ FailureOr<Attribute> query(DataLayoutEntryKey key);
+ }];
+}
+
+//===----------------------------------------------------------------------===//
+// LLVM_DataLayoutAttr
+//===----------------------------------------------------------------------===//
+
+def LLVM_DataLayoutAttr
+ : LLVM_Attr<"DataLayout", "data_layout", [DataLayoutSpecInterface]> {
+ let summary = "LLVM data layout string, exposed through DLTI";
+ let description = [{
+ An attribute to hold a LLVM data layout string.
+
+ The LLVM data layout string is parsed and mapped to the corresponding MLIR
+ data layout specification. The `#llvm.data_layout` attribute then serves as
+ a proxy, forwarding all DLTI queries to the underlying MLIR data layout
+ specification.
+ }];
+ let parameters = (ins "StringAttr":$data_layout_str,
+ OptionalParameter<"DataLayoutSpecInterface", "{}">:$data_layout_spec);
+ let builders = [
+ AttrBuilder<(ins "llvm::StringRef":$data_layout_str), [{
+ auto importer = LLVM::DataLayoutImporter($_ctxt, data_layout_str);
+ auto dataLayoutSpec = importer.getDataLayout();
+ return $_get($_ctxt, mlir::StringAttr::get($_ctxt, data_layout_str), dataLayoutSpec);
+ }]>
+ ];
+ let assemblyFormat = "`<` $data_layout_str `>`";
+ let extraClassDeclaration = [{
+ template <typename Ty>
+ DataLayoutEntryList getSpecForType() {
+ return getDataLayoutSpec().getSpecForType(TypeID::get<Ty>());
+ }
+
+ inline ::mlir::FailureOr<::mlir::Attribute>
+ queryHelper(::mlir::DataLayoutEntryKey key) const {
+ return getDataLayoutSpec().queryHelper(key);
+ }
+
+ void bucketEntriesByType(
+ ::llvm::MapVector<::mlir::TypeID, ::mlir::DataLayoutEntryList> &types,
+ ::llvm::MapVector<::mlir::StringAttr,
+ ::mlir::DataLayoutEntryInterface> &ids) {
+ getDataLayoutSpec().bucketEntriesByType(types, ids);
+ };
+
+ ::mlir::DataLayoutSpecInterface
+ combineWith(ArrayRef<::mlir::DataLayoutSpecInterface> specs) const {
+ return getDataLayoutSpec().combineWith(specs);
+ }
+ DataLayoutEntryListRef getEntries() const { return getDataLayoutSpec().getEntries(); }
+ LogicalResult verifySpec(Location loc) {
+ return getDataLayoutSpec().verifySpec(loc);
+ }
+ StringAttr getEndiannessIdentifier(MLIRContext *context) const {
+ return getDataLayoutSpec().getEndiannessIdentifier(context);
+ }
+ StringAttr getDefaultMemorySpaceIdentifier(MLIRContext *context) const {
+ return getDataLayoutSpec().getDefaultMemorySpaceIdentifier(context);
+ }
+ StringAttr getAllocaMemorySpaceIdentifier(MLIRContext *context) const {
+ return getDataLayoutSpec().getAllocaMemorySpaceIdentifier(context);
+ }
+ StringAttr getManglingModeIdentifier(MLIRContext *context) const {
+ return getDataLayoutSpec().getManglingModeIdentifier(context);
+ }
+ StringAttr getProgramMemorySpaceIdentifier(MLIRContext *context) const {
+ return getDataLayoutSpec().getProgramMemorySpaceIdentifier(context);
+ }
+ StringAttr getGlobalMemorySpaceIdentifier(MLIRContext *context) const {
+ return getDataLayoutSpec().getGlobalMemorySpaceIdentifier(context);
+ }
+ StringAttr getStackAlignmentIdentifier(MLIRContext *context) const {
+ return getDataLayoutSpec().getStackAlignmentIdentifier(context);
+ }
+ StringAttr getFunctionPointerAlignmentIdentifier(MLIRContext *context) const {
+ return getDataLayoutSpec().getFunctionPointerAlignmentIdentifier(context);
+ }
+ StringAttr getLegalIntWidthsIdentifier(MLIRContext *context) const {
+ return getDataLayoutSpec().getLegalIntWidthsIdentifier(context);
+ }
+ ::mlir::DataLayoutEntryList getSpecForType(TypeID type) const {
+ return getDataLayoutSpec().getSpecForType(type);
+ }
+ ::mlir::DataLayoutEntryInterface getSpecForIdentifier(StringAttr identifier) const {
+ return getDataLayoutSpec().getSpecForIdentifier(identifier);
+ }
+ FailureOr<Attribute> query(DataLayoutEntryKey key) const {
+ return getDataLayoutSpec().query(key);
+ }
+ }];
}
//===----------------------------------------------------------------------===//
@@ -1241,69 +1366,6 @@ def LLVM_VScaleRangeAttr : LLVM_Attr<"VScaleRange", "vscale_range"> {
let assemblyFormat = "`<` struct(params) `>`";
}
-//===----------------------------------------------------------------------===//
-// TargetFeaturesAttr
-//===----------------------------------------------------------------------===//
-
-def LLVM_TargetFeaturesAttr : LLVM_Attr<"TargetFeatures", "target_features">
-{
- let summary = "LLVM target features attribute";
-
- let description = [{
- Represents the LLVM target features as a list that can be checked within
- passes/rewrites.
-
- Example:
- ```mlir
- #llvm.target_features<["+sme", "+sve", "+sme-f64f64"]>
- ```
-
- Then within a pass or rewrite the features active at an op can be queried:
-
- ```c++
- auto targetFeatures = LLVM::TargetFeaturesAttr::featuresAt(op);
-
- if (!targetFeatures.contains("+sme-f64f64"))
- return failure();
- ```
- }];
-
- let parameters = (ins OptionalArrayRefParameter<"StringAttr">:$features);
-
- let builders = [
- TypeBuilder<(ins "::llvm::StringRef":$features)>,
- TypeBuilder<(ins "::llvm::ArrayRef<::llvm::StringRef>":$features)>
- ];
-
- let extraClassDeclaration = [{
- /// Checks if a feature is contained within the features list.
- /// Note: Using a StringAttr allows doing pointer-comparisons.
- bool contains(::mlir::StringAttr feature) const;
- bool contains(::llvm::StringRef feature) const;
-
- bool nullOrEmpty() const {
- // Checks if this attribute is null, or the features are empty.
- return !bool(*this) || getFeatures().empty();
- }
-
- /// Returns the list of features as an LLVM-compatible string.
- std::string getFeaturesString() const;
-
- /// Finds the target features on the parent FunctionOpInterface.
- /// Note: This assumes the attribute name matches the return value of
- /// `getAttributeName()`.
- static TargetFeaturesAttr featuresAt(Operation* op);
-
- /// Canonical name for this attribute within MLIR.
- static constexpr StringLiteral getAttributeName() {
- return StringLiteral("target_features");
- }
- }];
-
- let assemblyFormat = "`<` `[` (`]`) : ($features^ `]`)? `>`";
- let genVerifyDecl = 1;
-}
-
//===----------------------------------------------------------------------===//
// UndefAttr
//===----------------------------------------------------------------------===//
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrs.h b/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrs.h
index 3ede857733242..14645d5dee95f 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrs.h
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrs.h
@@ -14,8 +14,10 @@
#ifndef MLIR_DIALECT_LLVMIR_LLVMATTRS_H_
#define MLIR_DIALECT_LLVMIR_LLVMATTRS_H_
-#include "mlir/Dialect/LLVMIR/LLVMTypes.h"
#include "mlir/IR/OpImplementation.h"
+#include "mlir/Interfaces/DataLayoutInterfaces.h"
+#include "llvm/MC/TargetRegistry.h"
+#include "llvm/Target/TargetMachine.h"
#include <optional>
#include "mlir/Dialect/LLVMIR/LLVMOpsEnums.h.inc"
@@ -89,11 +91,17 @@ class TBAANodeAttr : public Attribute {
// TODO: this shouldn't be needed after we unify the attribute generation, i.e.
// --gen-attr-* and --gen-attrdef-*.
using cconv::CConv;
-using tailcallkind::TailCallKind;
using linkage::Linkage;
+using tailcallkind::TailCallKind;
} // namespace LLVM
} // namespace mlir
+// First obtain TargetFeaturesAttr definitions as it is used both an LLVMIR
+// interface and that interface and this attribute are turn required by another
+// LLVMIR attribute.
+#define GET_ATTRDEF_CLASSES
+#include "mlir/Dialect/LLVMIR/LLVMTargetFeaturesAttrDefs.h.inc"
+
#include "mlir/Dialect/LLVMIR/LLVMAttrInterfaces.h.inc"
#define GET_ATTRDEF_CLASSES
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMDialect.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMDialect.td
index b5ea8fc5da500..e924be32da10f 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMDialect.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMDialect.td
@@ -10,6 +10,7 @@
#define LLVMIR_DIALECT
include "mlir/IR/DialectBase.td"
+include "mlir/IR/AttrTypeBase.td"
def LLVM_Dialect : Dialect {
let name = "llvm";
@@ -123,4 +124,11 @@ def LLVM_Dialect : Dialect {
}];
}
+class LLVM_Attr<string name, string attrMnemonic,
+ list<Trait> traits = [],
+ string baseCppClass = "::mlir::Attribute">
+ : AttrDef<LLVM_Dialect, name, traits, baseCppClass> {
+ let mnemonic = attrMnemonic;
+}
+
#endif // LLVMIR_DIALECT
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMInterfaces.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMInterfaces.td
index 138170f8c8762..0d2603debbc28 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMInterfaces.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMInterfaces.td
@@ -14,6 +14,7 @@
#define LLVMIR_INTERFACES
include "mlir/IR/OpBase.td"
+include "mlir/Interfaces/DataLayoutInterfaces.td"
def FastmathFlagsInterface : OpInterface<"FastmathFlagsInterface"> {
let description = [{
@@ -532,4 +533,56 @@ def LLVM_DIRecursiveTypeAttrInterface
];
}
+def LLVM_TargetAttrInterface
+ : AttrInterface<"TargetAttrInterface", [DLTIQueryInterface]> {
+ let description = [{
+ Interface for attributes that describe LLVM targets.
+
+ These attributes should be able to return the specified target
+ `triple`, `chip` and `features` and are expected to be able to
+ produce the corresponding `llvm::TargetMachine` and
+ `llvm::DataLayout`. These methods can fail in case the backend
+ is not available.
+
+ Implementing attributes should provide a
+ `DLTIQueryInterface::query()` implementation which responds to
+ keys `"triple"`, `"chip"` and `"features"` by returning an
+ appropriate `StringAttr`, `StringAttr` and
+ `LLVM_TargetFeaturesAttr`.
+ }];
+ let cppNamespace = "::mlir::LLVM";
+ let methods = [
+ InterfaceMethod<
+ /*description=*/"Returns the target triple identifier.",
+ /*retTy=*/"::llvm::StringRef",
+ /*methodName=*/"getTriple",
+ /*args=*/(ins)
+ >,
+ InterfaceMethod<
+ /*description=*/"Returns the target chip (i.e. \"cpu\") identifier.",
+ /*retTy=*/"::llvm::StringRef",
+ /*methodName=*/"getChip",
+ /*args=*/(ins)
+ >,
+ InterfaceMethod<
+ /*description=*/"Returns the target features as a string.",
+ /*retTy=*/"::mlir::LLVM::TargetFeaturesAttr",
+ /*methodName=*/"getFeatures",
+ /*args=*/(ins)
+ >,
+ InterfaceMethod<
+ /*description=*/"Returns the target machine.",
+ /*retTy=*/"FailureOr<::llvm::TargetMachine *>",
+ /*methodName=*/"getTargetMachine",
+ /*args=*/(ins)
+ >,
+ InterfaceMethod<
+ /*description=*/"Returns the data layout associated to the target machine.",
+ /*retTy=*/"FailureOr<::llvm::DataLayout>",
+ /*methodName=*/"getDataLayout",
+ /*args=*/(ins)
+ >
+ ];
+}
+
#endif // LLVMIR_INTERFACES
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
index f4c1640098320..36a6291b5e3a8 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
@@ -14,6 +14,7 @@
#define LLVMIR_OPS
include "mlir/Dialect/LLVMIR/LLVMAttrDefs.td"
+include "mlir/Dialect/LLVMIR/LLVMTargetFeaturesAttrDefs.td"
include "mlir/Dialect/LLVMIR/LLVMEnums.td"
include...
[truncated]
|
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.
Nice! Quick comment, see my comment on LLVMAttrs.h
, that affects many things in this PR and should be fixed. I'll give it another look later in the week.
#include "llvm/MC/TargetRegistry.h" | ||
#include "llvm/Target/TargetMachine.h" |
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.
This is adding a lot of LLVM deps to the LLVM dialect that we don't want. The best way to remove this dep is adding a promised interface and implement LLVM_TargetAttrInterface
for that attr as an external model in mlir/lib/Target
.
Adds the
#llvm.target<triple = $TRIPLE, chip = $CHIP, #llvm.target_features<$FEATURES>>
attribute and#llvm.data_layout<DATA_LAYOUT_STR>
attribute with a-llvm-data-layout-from-target
pass to derive the latter from the former (using the existingDataLayoutImporter
). Both attributes implement the relevant DLTI-interfaces to expose thetriple
,chip
(AKAcpu
) andfeatures
(via nested DLTI queries on#llvm.target_features
) on#llvm.target
and the fullDataLayoutSpecInterface
on#llvm.data_layout
.#llvm.data_layout
converts into a (fully verbose)#dlti.dl_spec
in case it needs to combine with an already present#dlti.dl_spec
, e.g. one which specifies the index size.Adds a
TargetAttrInterface
which can be implemented by all attributes representing LLVM targets.Similar to the Draft PR #78073.
RFC on which this PR is based: https://discourse.llvm.org/t/mandatory-data-layout-in-the-llvm-dialect/85875