[Mlir-commits] [mlir] [mlir][llvm] Add llvm.target_features features attribute (PR #71510)
Benjamin Maxwell
llvmlistbot at llvm.org
Tue Dec 5 03:04:39 PST 2023
https://github.com/MacDue updated https://github.com/llvm/llvm-project/pull/71510
>From ae5834a7341184d47b6fa65ec6dbf3ed6c705a5e Mon Sep 17 00:00:00 2001
From: Benjamin Maxwell <benjamin.maxwell at arm.com>
Date: Tue, 7 Nov 2023 10:38:52 +0000
Subject: [PATCH 1/4] [mlir][llvm] Add llvm.target_features features attribute
This patch adds a target_features (TargetFeaturesAttr) to the LLVM
dialect to allow setting and querying the features in use on a function.
The features are stored as a sorted list rather plain string to allow
efficiently checking a function's features.
The motivation for this comes from the Arm SME dialect where we would
like a convenient way to check what variants of an operation are
available based on the CPU features.
Intended usage:
The target_features attribute is populated manually or by a pass:
```mlir
func.func @example() attributes {
target_features = #llvm.target_features<"+sme,+sve,+sme-f64f64">
} {
// ...
}
```
Then within a later rewrite the attribute can be checked, and used to
make lowering decisions.
```c++
// Finds the "target_features" attribute on the parent
// FunctionOpInterface.
auto targetFeatures = LLVM::TargetFeaturesAttr::featuresAt(op);
// Check a feature.
// Returns false if targetFeatures is null or the feature is not in
// the list.
if (!targetFeatures.contains("+sme-f64f64"))
return failure();
```
For now, this is rather simple just checks if the exact feature is in
the list, though it could be possible to extend with implied features
using information from LLVM.
---
.../mlir/Dialect/LLVMIR/LLVMAttrDefs.td | 53 ++++++++++++++
mlir/include/mlir/Dialect/LLVMIR/LLVMAttrs.h | 25 +++++++
mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td | 3 +-
mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp | 69 +++++++++++++++++++
mlir/lib/Target/LLVMIR/ModuleImport.cpp | 7 ++
mlir/lib/Target/LLVMIR/ModuleTranslation.cpp | 3 +
.../Target/LLVMIR/Import/target-features.ll | 9 +++
mlir/test/Target/LLVMIR/target-features.mlir | 7 ++
8 files changed, 175 insertions(+), 1 deletion(-)
create mode 100644 mlir/test/Target/LLVMIR/Import/target-features.ll
create mode 100644 mlir/test/Target/LLVMIR/target-features.mlir
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
index acbf88807b10c..61e2bc4f47c2f 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
@@ -933,4 +933,57 @@ def LLVM_VScaleRangeAttr : LLVM_Attr<"VScaleRange", "vscale_range"> {
"IntegerAttr":$maxRange);
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 in a manner that is efficient to query.
+
+ 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
+ ArrayRefOfSelfAllocationParameter<"TargetFeature", "">: $features);
+
+ let builders = [
+ TypeBuilder<(ins "::llvm::ArrayRef<TargetFeature>": $features)>,
+ TypeBuilder<(ins "StringRef": $features)>
+ ];
+
+ let extraClassDeclaration = [{
+ /// Checks if a feature is contained within the features list.
+ bool contains(TargetFeature) const;
+ bool contains(StringRef feature) const {
+ return contains(TargetFeature{feature});
+ }
+
+ /// 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 is called "target_features".
+ static TargetFeaturesAttr featuresAt(Operation* op);
+ }];
+
+ let hasCustomAssemblyFormat = 1;
+ let skipDefaultBuilders = 1;
+}
+
#endif // LLVMIR_ATTRDEFS
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrs.h b/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrs.h
index c370bfa2b733d..68a3d1f74175b 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrs.h
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrs.h
@@ -74,6 +74,31 @@ class TBAANodeAttr : public Attribute {
}
};
+/// This struct represents a single LLVM target feature.
+struct TargetFeature {
+ StringRef feature;
+
+ // Support allocating this struct into MLIR storage to ensure the feature
+ // string remains valid.
+ TargetFeature allocateInto(TypeStorageAllocator &alloc) const {
+ return TargetFeature{alloc.copyInto(feature)};
+ }
+
+ operator StringRef() const { return feature; }
+
+ bool operator==(TargetFeature const &other) const {
+ return feature == other.feature;
+ }
+
+ bool operator<(TargetFeature const &other) const {
+ return feature < other.feature;
+ }
+};
+
+inline llvm::hash_code hash_value(const TargetFeature &feature) {
+ return llvm::hash_value(feature.feature);
+}
+
// Inline the LLVM generated Linkage enum and utility.
// This is only necessary to isolate the "enum generated code" from the
// attribute definition itself.
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
index 83e71f071dec0..92460fa06f530 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
@@ -1394,7 +1394,8 @@ def LLVM_LLVMFuncOp : LLVM_Op<"func", [
OptionalAttr<UnnamedAddr>:$unnamed_addr,
OptionalAttr<I64Attr>:$alignment,
OptionalAttr<LLVM_VScaleRangeAttr>:$vscale_range,
- OptionalAttr<FramePointerKindAttr>:$frame_pointer
+ OptionalAttr<FramePointerKindAttr>:$frame_pointer,
+ OptionalAttr<LLVM_TargetFeaturesAttr>:$target_features
);
let regions = (region AnyRegion:$body);
diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp
index e2342670508ce..2498e7de42564 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp
@@ -14,11 +14,14 @@
#include "mlir/Dialect/LLVMIR/LLVMDialect.h"
#include "mlir/IR/Builders.h"
#include "mlir/IR/DialectImplementation.h"
+#include "mlir/Interfaces/FunctionInterfaces.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/ADT/TypeSwitch.h"
#include "llvm/BinaryFormat/Dwarf.h"
#include "llvm/IR/DebugInfoMetadata.h"
+#include <algorithm>
#include <optional>
+#include <set>
using namespace mlir;
using namespace mlir::LLVM;
@@ -183,3 +186,69 @@ void printExpressionArg(AsmPrinter &printer, uint64_t opcode,
i++;
});
}
+
+//===----------------------------------------------------------------------===//
+// TargetFeaturesAttr
+//===----------------------------------------------------------------------===//
+
+Attribute TargetFeaturesAttr::parse(mlir::AsmParser &parser, mlir::Type) {
+ std::string targetFeatures;
+ if (parser.parseLess() || parser.parseString(&targetFeatures) ||
+ parser.parseGreater())
+ return {};
+ return get(parser.getContext(), targetFeatures);
+}
+
+void TargetFeaturesAttr::print(mlir::AsmPrinter &printer) const {
+ printer << "<\"";
+ llvm::interleave(
+ getFeatures(), printer,
+ [&](auto &feature) { printer << StringRef(feature); }, ",");
+ printer << "\">";
+}
+
+TargetFeaturesAttr
+TargetFeaturesAttr::get(MLIRContext *context,
+ llvm::ArrayRef<TargetFeature> featuresRef) {
+ // Sort and de-duplicate target features.
+ std::set<TargetFeature> features(featuresRef.begin(), featuresRef.end());
+ return Base::get(context, llvm::to_vector(features));
+}
+
+TargetFeaturesAttr TargetFeaturesAttr::get(MLIRContext *context,
+ StringRef targetFeatures) {
+ SmallVector<StringRef> features;
+ StringRef{targetFeatures}.split(features, ',', /*MaxSplit=*/-1,
+ /*KeepEmpty=*/false);
+ return get(context, llvm::map_to_vector(features, [](StringRef feature) {
+ return TargetFeature{feature};
+ }));
+}
+
+bool TargetFeaturesAttr::contains(TargetFeature feature) const {
+ if (!bool(*this))
+ return false; // Allows checking null target features.
+ ArrayRef<TargetFeature> features = getFeatures();
+ // Note: The attribute getter ensures the feature list is sorted.
+ return std::binary_search(features.begin(), features.end(), feature);
+}
+
+std::string TargetFeaturesAttr::getFeaturesString() const {
+ std::string features;
+ bool first = true;
+ for (TargetFeature feature : getFeatures()) {
+ if (!first)
+ features += ",";
+ features += StringRef(feature);
+ first = false;
+ }
+ return features;
+}
+
+TargetFeaturesAttr TargetFeaturesAttr::featuresAt(Operation *op) {
+ auto parentFunction = op->getParentOfType<FunctionOpInterface>();
+ if (!parentFunction)
+ return {};
+ return parentFunction.getOperation()->getAttrOfType<TargetFeaturesAttr>(
+ "target_features");
+}
diff --git a/mlir/lib/Target/LLVMIR/ModuleImport.cpp b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
index a7548ea9b9849..4bdffa572e31a 100644
--- a/mlir/lib/Target/LLVMIR/ModuleImport.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
@@ -1627,6 +1627,7 @@ static constexpr std::array ExplicitAttributes{
StringLiteral("aarch64_pstate_za_new"),
StringLiteral("vscale_range"),
StringLiteral("frame-pointer"),
+ StringLiteral("target-features"),
};
static void processPassthroughAttrs(llvm::Function *func, LLVMFuncOp funcOp) {
@@ -1717,6 +1718,12 @@ void ModuleImport::processFunctionAttributes(llvm::Function *func,
stringRefFramePointerKind)
.value()));
}
+
+ if (llvm::Attribute attr = func->getFnAttribute("target-features");
+ attr.isStringAttribute()) {
+ funcOp.setTargetFeaturesAttr(
+ LLVM::TargetFeaturesAttr::get(context, attr.getValueAsString()));
+ }
}
DictionaryAttr
diff --git a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
index 5e94fa6acf7b9..d6afe354178d6 100644
--- a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
@@ -968,6 +968,9 @@ LogicalResult ModuleTranslation::convertOneFunction(LLVMFuncOp func) {
if (func.getArmNewZa())
llvmFunc->addFnAttr("aarch64_pstate_za_new");
+ if (auto targetFeatures = func.getTargetFeatures())
+ llvmFunc->addFnAttr("target-features", targetFeatures->getFeaturesString());
+
if (auto attr = func.getVscaleRange())
llvmFunc->addFnAttr(llvm::Attribute::getWithVScaleRangeArgs(
getLLVMContext(), attr->getMinRange().getInt(),
diff --git a/mlir/test/Target/LLVMIR/Import/target-features.ll b/mlir/test/Target/LLVMIR/Import/target-features.ll
new file mode 100644
index 0000000000000..39e9a1204d3e0
--- /dev/null
+++ b/mlir/test/Target/LLVMIR/Import/target-features.ll
@@ -0,0 +1,9 @@
+; RUN: mlir-translate -import-llvm -split-input-file %s | FileCheck %s
+
+; CHECK-LABEL: llvm.func @target_features()
+; CHECK-SAME: #llvm.target_features<"+sme,+sme-f64f64,+sve">
+define void @target_features() #0 {
+ ret void
+}
+
+attributes #0 = { "target-features"="+sme,+sme-f64f64,+sve" }
diff --git a/mlir/test/Target/LLVMIR/target-features.mlir b/mlir/test/Target/LLVMIR/target-features.mlir
new file mode 100644
index 0000000000000..02c07d27ca3cd
--- /dev/null
+++ b/mlir/test/Target/LLVMIR/target-features.mlir
@@ -0,0 +1,7 @@
+// RUN: mlir-translate -mlir-to-llvmir %s | FileCheck %s
+
+// CHECK-LABEL: define void @target_features
+// CHECK: attributes #{{.*}} = { "target-features"="+sme,+sme-f64f64,+sve" }
+llvm.func @target_features() attributes { target_features = #llvm.target_features<"+sme,+sve,+sme-f64f64"> } {
+ llvm.return
+}
>From b0a46cb0406530c33bb16fd1b0957714324bcbe7 Mon Sep 17 00:00:00 2001
From: Benjamin Maxwell <benjamin.maxwell at arm.com>
Date: Tue, 7 Nov 2023 12:21:26 +0000
Subject: [PATCH 2/4] Fixups
---
.../mlir/Dialect/LLVMIR/LLVMAttrDefs.td | 11 +++++++---
mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp | 20 ++++++++-----------
2 files changed, 16 insertions(+), 15 deletions(-)
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
index 61e2bc4f47c2f..8e0e045554f12 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
@@ -960,11 +960,11 @@ def LLVM_TargetFeaturesAttr : LLVM_Attr<"TargetFeatures", "target_features"> {
}];
let parameters = (ins
- ArrayRefOfSelfAllocationParameter<"TargetFeature", "">: $features);
+ ArrayRefOfSelfAllocationParameter<"TargetFeature", "">:$features);
let builders = [
- TypeBuilder<(ins "::llvm::ArrayRef<TargetFeature>": $features)>,
- TypeBuilder<(ins "StringRef": $features)>
+ TypeBuilder<(ins "::llvm::ArrayRef<TargetFeature>":$features)>,
+ TypeBuilder<(ins "::llvm::StringRef":$features)>
];
let extraClassDeclaration = [{
@@ -980,6 +980,11 @@ def LLVM_TargetFeaturesAttr : LLVM_Attr<"TargetFeatures", "target_features"> {
/// Finds the target features on the parent FunctionOpInterface.
/// Note: This assumes the attribute is called "target_features".
static TargetFeaturesAttr featuresAt(Operation* op);
+
+ /// Canonical name for this attribute within MLIR.
+ static constexpr StringLiteral attributeName() {
+ return StringLiteral("target_features");
+ }
}];
let hasCustomAssemblyFormat = 1;
diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp
index 2498e7de42564..fc49b30923507 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp
@@ -218,8 +218,8 @@ TargetFeaturesAttr::get(MLIRContext *context,
TargetFeaturesAttr TargetFeaturesAttr::get(MLIRContext *context,
StringRef targetFeatures) {
SmallVector<StringRef> features;
- StringRef{targetFeatures}.split(features, ',', /*MaxSplit=*/-1,
- /*KeepEmpty=*/false);
+ targetFeatures.split(features, ',', /*MaxSplit=*/-1,
+ /*KeepEmpty=*/false);
return get(context, llvm::map_to_vector(features, [](StringRef feature) {
return TargetFeature{feature};
}));
@@ -234,15 +234,11 @@ bool TargetFeaturesAttr::contains(TargetFeature feature) const {
}
std::string TargetFeaturesAttr::getFeaturesString() const {
- std::string features;
- bool first = true;
- for (TargetFeature feature : getFeatures()) {
- if (!first)
- features += ",";
- features += StringRef(feature);
- first = false;
- }
- return features;
+ std::string featuresString;
+ llvm::raw_string_ostream ss(featuresString);
+ llvm::interleave(
+ getFeatures(), ss, [&](auto &feature) { ss << StringRef(feature); }, ",");
+ return ss.str();
}
TargetFeaturesAttr TargetFeaturesAttr::featuresAt(Operation *op) {
@@ -250,5 +246,5 @@ TargetFeaturesAttr TargetFeaturesAttr::featuresAt(Operation *op) {
if (!parentFunction)
return {};
return parentFunction.getOperation()->getAttrOfType<TargetFeaturesAttr>(
- "target_features");
+ attributeName());
}
>From 38ac08d91ba5e4c66e8a14ea151fab0979aef0d2 Mon Sep 17 00:00:00 2001
From: Benjamin Maxwell <benjamin.maxwell at arm.com>
Date: Wed, 8 Nov 2023 10:28:53 +0000
Subject: [PATCH 3/4] Switch to using a list of StringAttr
---
.../mlir/Dialect/LLVMIR/LLVMAttrDefs.td | 29 ++++----
mlir/include/mlir/Dialect/LLVMIR/LLVMAttrs.h | 25 -------
mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp | 68 +++++++++----------
.../Target/LLVMIR/Import/target-features.ll | 2 +-
mlir/test/Target/LLVMIR/llvmir-invalid.mlir | 21 ++++++
mlir/test/Target/LLVMIR/target-features.mlir | 6 +-
6 files changed, 77 insertions(+), 74 deletions(-)
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
index 8e0e045554f12..bb061655ceea7 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
@@ -938,15 +938,17 @@ def LLVM_VScaleRangeAttr : LLVM_Attr<"VScaleRange", "vscale_range"> {
// TargetFeaturesAttr
//===----------------------------------------------------------------------===//
-def LLVM_TargetFeaturesAttr : LLVM_Attr<"TargetFeatures", "target_features"> {
+def LLVM_TargetFeaturesAttr : LLVM_Attr<"TargetFeatures", "target_features">
+{
let summary = "LLVM target features attribute";
let description = [{
- Represents the LLVM target features in a manner that is efficient to query.
+ Represents the LLVM target features as a list that can be checked within
+ passes/rewrites.
Example:
```mlir
- #llvm.target_features<"+sme,+sve,+sme-f64f64">
+ #llvm.target_features<["+sme", "+sve", "+sme-f64f64"]>
```
Then within a pass or rewrite the features active at an op can be queried:
@@ -959,19 +961,22 @@ def LLVM_TargetFeaturesAttr : LLVM_Attr<"TargetFeatures", "target_features"> {
```
}];
- let parameters = (ins
- ArrayRefOfSelfAllocationParameter<"TargetFeature", "">:$features);
+ let parameters = (ins OptionalArrayRefParameter<"StringAttr">:$features);
let builders = [
- TypeBuilder<(ins "::llvm::ArrayRef<TargetFeature>":$features)>,
- TypeBuilder<(ins "::llvm::StringRef":$features)>
+ TypeBuilder<(ins "::llvm::StringRef":$features)>,
+ TypeBuilder<(ins "::llvm::ArrayRef<::llvm::StringRef>":$features)>
];
let extraClassDeclaration = [{
/// Checks if a feature is contained within the features list.
- bool contains(TargetFeature) const;
- bool contains(StringRef feature) const {
- return contains(TargetFeature{feature});
+ /// 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 fasly, or the features are empty.
+ return !bool(*this) || getFeatures().empty();
}
/// Returns the list of features as an LLVM-compatible string.
@@ -987,8 +992,8 @@ def LLVM_TargetFeaturesAttr : LLVM_Attr<"TargetFeatures", "target_features"> {
}
}];
- let hasCustomAssemblyFormat = 1;
- let skipDefaultBuilders = 1;
+ let assemblyFormat = "`<` `[` (`]`) : ($features^ `]`)? `>`";
+ let genVerifyDecl = 1;
}
#endif // LLVMIR_ATTRDEFS
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrs.h b/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrs.h
index 68a3d1f74175b..c370bfa2b733d 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrs.h
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrs.h
@@ -74,31 +74,6 @@ class TBAANodeAttr : public Attribute {
}
};
-/// This struct represents a single LLVM target feature.
-struct TargetFeature {
- StringRef feature;
-
- // Support allocating this struct into MLIR storage to ensure the feature
- // string remains valid.
- TargetFeature allocateInto(TypeStorageAllocator &alloc) const {
- return TargetFeature{alloc.copyInto(feature)};
- }
-
- operator StringRef() const { return feature; }
-
- bool operator==(TargetFeature const &other) const {
- return feature == other.feature;
- }
-
- bool operator<(TargetFeature const &other) const {
- return feature < other.feature;
- }
-};
-
-inline llvm::hash_code hash_value(const TargetFeature &feature) {
- return llvm::hash_value(feature.feature);
-}
-
// Inline the LLVM generated Linkage enum and utility.
// This is only necessary to isolate the "enum generated code" from the
// attribute definition itself.
diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp
index fc49b30923507..5f9fbfebbb7a0 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp
@@ -19,9 +19,7 @@
#include "llvm/ADT/TypeSwitch.h"
#include "llvm/BinaryFormat/Dwarf.h"
#include "llvm/IR/DebugInfoMetadata.h"
-#include <algorithm>
#include <optional>
-#include <set>
using namespace mlir;
using namespace mlir::LLVM;
@@ -191,28 +189,12 @@ void printExpressionArg(AsmPrinter &printer, uint64_t opcode,
// TargetFeaturesAttr
//===----------------------------------------------------------------------===//
-Attribute TargetFeaturesAttr::parse(mlir::AsmParser &parser, mlir::Type) {
- std::string targetFeatures;
- if (parser.parseLess() || parser.parseString(&targetFeatures) ||
- parser.parseGreater())
- return {};
- return get(parser.getContext(), targetFeatures);
-}
-
-void TargetFeaturesAttr::print(mlir::AsmPrinter &printer) const {
- printer << "<\"";
- llvm::interleave(
- getFeatures(), printer,
- [&](auto &feature) { printer << StringRef(feature); }, ",");
- printer << "\">";
-}
-
-TargetFeaturesAttr
-TargetFeaturesAttr::get(MLIRContext *context,
- llvm::ArrayRef<TargetFeature> featuresRef) {
- // Sort and de-duplicate target features.
- std::set<TargetFeature> features(featuresRef.begin(), featuresRef.end());
- return Base::get(context, llvm::to_vector(features));
+TargetFeaturesAttr TargetFeaturesAttr::get(MLIRContext *context,
+ llvm::ArrayRef<StringRef> features) {
+ return Base::get(context,
+ llvm::map_to_vector(features, [&](StringRef feature) {
+ return StringAttr::get(context, feature);
+ }));
}
TargetFeaturesAttr TargetFeaturesAttr::get(MLIRContext *context,
@@ -220,24 +202,42 @@ TargetFeaturesAttr TargetFeaturesAttr::get(MLIRContext *context,
SmallVector<StringRef> features;
targetFeatures.split(features, ',', /*MaxSplit=*/-1,
/*KeepEmpty=*/false);
- return get(context, llvm::map_to_vector(features, [](StringRef feature) {
- return TargetFeature{feature};
- }));
+ return get(context, features);
+}
+
+LogicalResult
+TargetFeaturesAttr::verify(function_ref<InFlightDiagnostic()> emitError,
+ llvm::ArrayRef<StringAttr> features) {
+ for (StringAttr featureAttr : features) {
+ if (!featureAttr || featureAttr.empty())
+ return emitError() << "target features can not be null or empty";
+ auto feature = featureAttr.strref();
+ if (feature[0] != '+' && feature[0] != '-')
+ return emitError() << "target features must start with '+' or '-'";
+ if (feature.contains(','))
+ return emitError() << "target features can not contain ','";
+ }
+ return success();
+}
+
+bool TargetFeaturesAttr::contains(StringAttr feature) const {
+ if (nullOrEmpty())
+ return false;
+ // Note: Using StringAttr does pointer comparisons.
+ return llvm::is_contained(getFeatures(), feature);
}
-bool TargetFeaturesAttr::contains(TargetFeature feature) const {
- if (!bool(*this))
- return false; // Allows checking null target features.
- ArrayRef<TargetFeature> features = getFeatures();
- // Note: The attribute getter ensures the feature list is sorted.
- return std::binary_search(features.begin(), features.end(), feature);
+bool TargetFeaturesAttr::contains(StringRef feature) const {
+ if (nullOrEmpty())
+ return false;
+ return llvm::is_contained(getFeatures(), feature);
}
std::string TargetFeaturesAttr::getFeaturesString() const {
std::string featuresString;
llvm::raw_string_ostream ss(featuresString);
llvm::interleave(
- getFeatures(), ss, [&](auto &feature) { ss << StringRef(feature); }, ",");
+ getFeatures(), ss, [&](auto &feature) { ss << feature.strref(); }, ",");
return ss.str();
}
diff --git a/mlir/test/Target/LLVMIR/Import/target-features.ll b/mlir/test/Target/LLVMIR/Import/target-features.ll
index 39e9a1204d3e0..d3feeda85691a 100644
--- a/mlir/test/Target/LLVMIR/Import/target-features.ll
+++ b/mlir/test/Target/LLVMIR/Import/target-features.ll
@@ -1,7 +1,7 @@
; RUN: mlir-translate -import-llvm -split-input-file %s | FileCheck %s
; CHECK-LABEL: llvm.func @target_features()
-; CHECK-SAME: #llvm.target_features<"+sme,+sme-f64f64,+sve">
+; CHECK-SAME: #llvm.target_features<["+sme", "+sme-f64f64", "+sve"]>
define void @target_features() #0 {
ret void
}
diff --git a/mlir/test/Target/LLVMIR/llvmir-invalid.mlir b/mlir/test/Target/LLVMIR/llvmir-invalid.mlir
index 0def5895fb330..521d94c45890a 100644
--- a/mlir/test/Target/LLVMIR/llvmir-invalid.mlir
+++ b/mlir/test/Target/LLVMIR/llvmir-invalid.mlir
@@ -261,6 +261,27 @@ llvm.func @stepvector_intr_wrong_type() -> vector<7xf32> {
// -----
+// expected-error @below{{target features can not contain ','}}
+llvm.func @invalid_target_feature() attributes { target_features = #llvm.target_features<["+bad,feature", "+test"]> }
+{
+}
+
+// -----
+
+// expected-error @below{{target features must start with '+' or '-'}}
+llvm.func @missing_target_feature_prefix() attributes { target_features = #llvm.target_features<["sme"]> }
+{
+}
+
+// -----
+
+// expected-error @below{{target features can not be null or empty}}
+llvm.func @empty_target_feature() attributes { target_features = #llvm.target_features<["", "+sve"]> }
+{
+}
+
+// -----
+
llvm.comdat @__llvm_comdat {
llvm.comdat_selector @foo any
}
diff --git a/mlir/test/Target/LLVMIR/target-features.mlir b/mlir/test/Target/LLVMIR/target-features.mlir
index 02c07d27ca3cd..7a69a2c788978 100644
--- a/mlir/test/Target/LLVMIR/target-features.mlir
+++ b/mlir/test/Target/LLVMIR/target-features.mlir
@@ -1,7 +1,9 @@
// RUN: mlir-translate -mlir-to-llvmir %s | FileCheck %s
// CHECK-LABEL: define void @target_features
-// CHECK: attributes #{{.*}} = { "target-features"="+sme,+sme-f64f64,+sve" }
-llvm.func @target_features() attributes { target_features = #llvm.target_features<"+sme,+sve,+sme-f64f64"> } {
+// CHECK: attributes #{{.*}} = { "target-features"="+sme,+sve,+sme-f64f64" }
+llvm.func @target_features() attributes {
+ target_features = #llvm.target_features<["+sme", "+sve", "+sme-f64f64"]>
+} {
llvm.return
}
>From bea4aafe08734b8ea981c6f4488f1d5b51bc596c Mon Sep 17 00:00:00 2001
From: Benjamin Maxwell <benjamin.maxwell at arm.com>
Date: Fri, 1 Dec 2023 12:44:03 +0000
Subject: [PATCH 4/4] Fixups
---
mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td | 7 ++++---
mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp | 2 +-
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
index bb061655ceea7..6975b18ab7f81 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
@@ -975,7 +975,7 @@ def LLVM_TargetFeaturesAttr : LLVM_Attr<"TargetFeatures", "target_features">
bool contains(::llvm::StringRef feature) const;
bool nullOrEmpty() const {
- // Checks if this attribute is fasly, or the features are empty.
+ // Checks if this attribute is null, or the features are empty.
return !bool(*this) || getFeatures().empty();
}
@@ -983,11 +983,12 @@ def LLVM_TargetFeaturesAttr : LLVM_Attr<"TargetFeatures", "target_features">
std::string getFeaturesString() const;
/// Finds the target features on the parent FunctionOpInterface.
- /// Note: This assumes the attribute is called "target_features".
+ /// 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 attributeName() {
+ static constexpr StringLiteral getAttributeName() {
return StringLiteral("target_features");
}
}];
diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp
index 5f9fbfebbb7a0..645a45dd96bef 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp
@@ -246,5 +246,5 @@ TargetFeaturesAttr TargetFeaturesAttr::featuresAt(Operation *op) {
if (!parentFunction)
return {};
return parentFunction.getOperation()->getAttrOfType<TargetFeaturesAttr>(
- attributeName());
+ getAttributeName());
}
More information about the Mlir-commits
mailing list