[Mlir-commits] [flang] [mlir] [MLIR][DLTI] Pretty parsing and printing for DLTI attrs (PR #113365)

Rolf Morel llvmlistbot at llvm.org
Wed Oct 30 12:23:17 PDT 2024


https://github.com/rolfmorel updated https://github.com/llvm/llvm-project/pull/113365

>From 75a73d8a352629879606adecd78a0a41e42b51b4 Mon Sep 17 00:00:00 2001
From: Rolf Morel <rolf.morel at intel.com>
Date: Tue, 20 Aug 2024 01:59:57 -0700
Subject: [PATCH 1/3] [MLIR][DLTI] Pretty parsing and printing for DLTI attrs

Unifies parsing and printing for DLTI attributes. Introduces syntax of
`#dlti.attr<key1 = val1, ..., keyN = valN>` for all queryable DLTI
 attributes, while retaining support for specifying key-value entry
pairs with `#dlti.dl_entry` (whether to retain this is TBD).

As the new format does away with much of the boilerplate, it is much
easier on the eye. This makes an especially big difference for nested
attributes.

Updates the DLTI tests and includes fixes for misc error checking/
error messages.
---
 mlir/include/mlir/Dialect/DLTI/DLTIAttrs.td   |  50 +--
 .../mlir/Interfaces/DataLayoutInterfaces.h    |   6 +-
 .../mlir/Interfaces/DataLayoutInterfaces.td   |   2 +-
 mlir/lib/Dialect/DLTI/DLTI.cpp                | 320 +++++++++++-------
 mlir/lib/Interfaces/DataLayoutInterfaces.cpp  |  13 +-
 mlir/test/Dialect/DLTI/invalid.mlir           |  37 +-
 mlir/test/Dialect/DLTI/query.mlir             | 103 +++---
 mlir/test/Dialect/DLTI/roundtrip.mlir         |  50 +--
 mlir/test/Dialect/DLTI/valid.mlir             | 217 +++++++-----
 mlir/test/Dialect/GPU/outlining.mlir          |  26 +-
 mlir/test/Target/LLVMIR/Import/data-layout.ll |  48 +--
 .../Interfaces/DataLayoutInterfacesTest.cpp   |  38 ++-
 12 files changed, 510 insertions(+), 400 deletions(-)

diff --git a/mlir/include/mlir/Dialect/DLTI/DLTIAttrs.td b/mlir/include/mlir/Dialect/DLTI/DLTIAttrs.td
index 53d38407608bed..1caf5fd8787c7b 100644
--- a/mlir/include/mlir/Dialect/DLTI/DLTIAttrs.td
+++ b/mlir/include/mlir/Dialect/DLTI/DLTIAttrs.td
@@ -93,6 +93,10 @@ def DLTI_DataLayoutSpecAttr :
   }];
 }
 
+//===----------------------------------------------------------------------===//
+// MapAttr
+//===----------------------------------------------------------------------===//
+
 def DLTI_MapAttr : DLTIAttr<"Map", [DLTIQueryInterface]> {
   let summary = "A mapping of DLTI-information by way of key-value pairs";
   let description = [{
@@ -106,18 +110,16 @@ def DLTI_MapAttr : DLTIAttr<"Map", [DLTIQueryInterface]> {
 
     Consider the following flat encoding of a single-key dictionary
     ```
-    #dlti.map<#dlti.dl_entry<"CPU::cache::L1::size_in_bytes", 65536 : i32>>
+    #dlti.map<"CPU::cache::L1::size_in_bytes" = 65536 : i32>>
     ```
     versus nested maps, which make it possible to obtain sub-dictionaries of
     related information (with the following example making use of other
     attributes that also implement the `DLTIQueryInterface`):
     ```
-    #dlti.target_system_spec<"CPU":
-      #dlti.target_device_spec<#dlti.dl_entry<"cache",
-        #dlti.map<#dlti.dl_entry<"L1",
-          #dlti.map<#dlti.dl_entry<"size_in_bytes", 65536 : i32>>>,
-                  #dlti.dl_entry<"L1d",
-          #dlti.map<#dlti.dl_entry<"size_in_bytes", 32768 : i32>>> >>>>
+    #dlti.target_system_spec<"CPU" =
+      #dlti.target_device_spec<"cache" =
+        #dlti.map<"L1" = #dlti.map<"size_in_bytes" = 65536 : i32>,
+                  "L1d" = #dlti.map<"size_in_bytes" = 32768 : i32> >>>
     ```
 
     With the flat encoding, the implied structure of the key is ignored, that is
@@ -139,7 +141,7 @@ def DLTI_MapAttr : DLTIAttr<"Map", [DLTIQueryInterface]> {
   );
   let mnemonic = "map";
   let genVerifyDecl = 1;
-  let assemblyFormat = "`<` $entries `>`";
+  let hasCustomAssemblyFormat = 1;
   let extraClassDeclaration = [{
     /// Returns the attribute associated with the key.
     FailureOr<Attribute> query(DataLayoutEntryKey key) {
@@ -167,20 +169,23 @@ def DLTI_TargetSystemSpecAttr :
     ```
     dlti.target_system_spec =
      #dlti.target_system_spec<
-      "CPU": #dlti.target_device_spec<
-              #dlti.dl_entry<"dlti.L1_cache_size_in_bytes", 4096: ui32>>,
-      "GPU": #dlti.target_device_spec<
-              #dlti.dl_entry<"dlti.max_vector_op_width", 64 : ui32>>,
-      "XPU": #dlti.target_device_spec<
-              #dlti.dl_entry<"dlti.max_vector_op_width", 4096 : ui32>>>
+      "CPU" = #dlti.target_device_spec<
+        "L1_cache_size_in_bytes" = 4096: ui32>,
+      "GPU" = #dlti.target_device_spec<
+        "max_vector_op_width" = 64 : ui32>,
+      "XPU" = #dlti.target_device_spec<
+        "max_vector_op_width" = 4096 : ui32>>
     ```
+
+    The verifier checks that keys are strings and pointed to values implement
+    DLTI's TargetDeviceSpecInterface.
   }];
   let parameters = (ins
-    ArrayRefParameter<"DeviceIDTargetDeviceSpecPair", "">:$entries
+    ArrayRefParameter<"DataLayoutEntryInterface">:$entries
   );
   let mnemonic = "target_system_spec";
   let genVerifyDecl = 1;
-  let assemblyFormat = "`<` $entries `>`";
+  let hasCustomAssemblyFormat = 1;
   let extraClassDeclaration = [{
     /// Return the device specification that matches the given device ID
     std::optional<TargetDeviceSpecInterface>
@@ -197,8 +202,10 @@ def DLTI_TargetSystemSpecAttr :
     $cppClass::getDeviceSpecForDeviceID(
         TargetSystemSpecInterface::DeviceID deviceID) {
       for (const auto& entry : getEntries()) {
-        if (entry.first == deviceID)
-          return entry.second;
+        if (entry.getKey() == DataLayoutEntryKey(deviceID))
+          if (auto deviceSpec =
+              llvm::dyn_cast<TargetDeviceSpecInterface>(entry.getValue()))
+            return deviceSpec;
       }
       return std::nullopt;
     }
@@ -219,16 +226,15 @@ def DLTI_TargetDeviceSpecAttr :
 
     Example:
     ```
-    #dlti.target_device_spec<
-      #dlti.dl_entry<"dlti.max_vector_op_width", 64 : ui32>>
+    #dlti.target_device_spec<"max_vector_op_width" = 64 : ui32>
     ```
   }];
   let parameters = (ins
-    ArrayRefParameter<"DataLayoutEntryInterface", "">:$entries
+    ArrayRefParameter<"DataLayoutEntryInterface">:$entries
   );
   let mnemonic = "target_device_spec";
   let genVerifyDecl = 1;
-  let assemblyFormat = "`<` $entries `>`";
+  let hasCustomAssemblyFormat = 1;
 
   let extraClassDeclaration = [{
     /// Returns the attribute associated with the key.
diff --git a/mlir/include/mlir/Interfaces/DataLayoutInterfaces.h b/mlir/include/mlir/Interfaces/DataLayoutInterfaces.h
index 848d2dee4a6309..7a7b659724f860 100644
--- a/mlir/include/mlir/Interfaces/DataLayoutInterfaces.h
+++ b/mlir/include/mlir/Interfaces/DataLayoutInterfaces.h
@@ -15,6 +15,7 @@
 #ifndef MLIR_INTERFACES_DATALAYOUTINTERFACES_H
 #define MLIR_INTERFACES_DATALAYOUTINTERFACES_H
 
+#include "mlir/IR/Attributes.h"
 #include "mlir/IR/DialectInterface.h"
 #include "mlir/IR/OpDefinition.h"
 #include "llvm/ADT/DenseMap.h"
@@ -32,10 +33,7 @@ using DataLayoutEntryKey = llvm::PointerUnion<Type, StringAttr>;
 using DataLayoutEntryList = llvm::SmallVector<DataLayoutEntryInterface, 4>;
 using DataLayoutEntryListRef = llvm::ArrayRef<DataLayoutEntryInterface>;
 using TargetDeviceSpecListRef = llvm::ArrayRef<TargetDeviceSpecInterface>;
-using DeviceIDTargetDeviceSpecPair =
-    std::pair<StringAttr, TargetDeviceSpecInterface>;
-using DeviceIDTargetDeviceSpecPairListRef =
-    llvm::ArrayRef<DeviceIDTargetDeviceSpecPair>;
+using TargetDeviceSpecEntry = std::pair<StringAttr, TargetDeviceSpecInterface>;
 class DataLayoutOpInterface;
 class DataLayoutSpecInterface;
 class ModuleOp;
diff --git a/mlir/include/mlir/Interfaces/DataLayoutInterfaces.td b/mlir/include/mlir/Interfaces/DataLayoutInterfaces.td
index d6e955be4291a3..061dee2399d9ad 100644
--- a/mlir/include/mlir/Interfaces/DataLayoutInterfaces.td
+++ b/mlir/include/mlir/Interfaces/DataLayoutInterfaces.td
@@ -304,7 +304,7 @@ def TargetSystemSpecInterface : AttrInterface<"TargetSystemSpecInterface", [DLTI
   let methods = [
     InterfaceMethod<
       /*description=*/"Returns the list of layout entries.",
-      /*retTy=*/"llvm::ArrayRef<DeviceIDTargetDeviceSpecPair>",
+      /*retTy=*/"llvm::ArrayRef<DataLayoutEntryInterface>",
       /*methodName=*/"getEntries",
       /*args=*/(ins)
     >,
diff --git a/mlir/lib/Dialect/DLTI/DLTI.cpp b/mlir/lib/Dialect/DLTI/DLTI.cpp
index 85ec9fc93248a1..d8946d865a1836 100644
--- a/mlir/lib/Dialect/DLTI/DLTI.cpp
+++ b/mlir/lib/Dialect/DLTI/DLTI.cpp
@@ -8,6 +8,7 @@
 
 #include "mlir/Dialect/DLTI/DLTI.h"
 #include "mlir/IR/Builders.h"
+#include "mlir/IR/BuiltinAttributes.h"
 #include "mlir/IR/BuiltinDialect.h"
 #include "mlir/IR/BuiltinOps.h"
 #include "mlir/IR/BuiltinTypes.h"
@@ -28,6 +29,123 @@ using namespace mlir;
 
 #define DEBUG_TYPE "dlti"
 
+//===----------------------------------------------------------------------===//
+// parsing
+//===----------------------------------------------------------------------===//
+
+static ParseResult parseKeyValuePair(AsmParser &parser,
+                                     DataLayoutEntryInterface &entry,
+                                     bool tryType = false) {
+  Attribute value;
+
+  if (tryType) {
+    Type type;
+    OptionalParseResult parsedType = parser.parseOptionalType(type);
+    if (parsedType.has_value()) {
+      if (failed(parsedType.value()))
+        return parser.emitError(parser.getCurrentLocation())
+               << "error while parsing type DLTI key";
+
+      if (failed(parser.parseEqual()) || failed(parser.parseAttribute(value)))
+        return failure();
+
+      entry = DataLayoutEntryAttr::get(type, value);
+      return ParseResult::success();
+    }
+  }
+
+  std::string ident;
+  OptionalParseResult parsedStr = parser.parseOptionalString(&ident);
+  if (parsedStr.has_value() && !ident.empty()) {
+    if (failed(parsedStr.value()))
+      return parser.emitError(parser.getCurrentLocation())
+             << "error while parsing string DLTI key";
+
+    if (failed(parser.parseEqual()) || failed(parser.parseAttribute(value)))
+      return failure(); // Assume that an error has already been emitted.
+
+    entry = DataLayoutEntryAttr::get(
+        StringAttr::get(parser.getContext(), ident), value);
+    return ParseResult::success();
+  }
+
+  OptionalParseResult parsedEntry = parser.parseAttribute(entry);
+  if (parsedEntry.has_value()) {
+    if (succeeded(parsedEntry.value()))
+      return parsedEntry.value();
+    return failure(); // Assume that an error has already been emitted.
+  }
+  return parser.emitError(parser.getCurrentLocation())
+         << "failed to parse DLTI entry";
+}
+
+template <class Attr>
+static Attribute parseAngleBracketedEntries(AsmParser &parser, Type ty,
+                                            bool tryType = false,
+                                            bool allowEmpty = false) {
+  SmallVector<DataLayoutEntryInterface> entries;
+  if (failed(parser.parseCommaSeparatedList(
+          AsmParser::Delimiter::LessGreater, [&]() {
+            return parseKeyValuePair(parser, entries.emplace_back(), tryType);
+          })))
+    return {};
+
+  if (entries.empty() && !allowEmpty) {
+    parser.emitError(parser.getNameLoc()) << "no DLTI entries provided";
+    return {};
+  }
+
+  return Attr::getChecked([&] { return parser.emitError(parser.getNameLoc()); },
+                          parser.getContext(), ArrayRef(entries));
+}
+
+//===----------------------------------------------------------------------===//
+// printing
+//===----------------------------------------------------------------------===//
+
+static inline std::string keyToStr(DataLayoutEntryKey key) {
+  std::string buf;
+  llvm::TypeSwitch<DataLayoutEntryKey>(key)
+      .Case<StringAttr, Type>( // The only two kinds of key we know of.
+          [&](auto key) { llvm::raw_string_ostream(buf) << key; })
+      .Default([](auto) { llvm_unreachable("unexpected entry key kind"); });
+  return buf;
+}
+
+template <class T>
+static void printAngleBracketedEntries(AsmPrinter &os, T &&entries) {
+  os << "<";
+  llvm::interleaveComma(std::forward<T>(entries), os, [&](auto entry) {
+    os << keyToStr(entry.getKey()) << " = " << entry.getValue();
+  });
+  os << ">";
+}
+
+//===----------------------------------------------------------------------===//
+// verifying
+//===----------------------------------------------------------------------===//
+
+static LogicalResult verifyEntries(function_ref<InFlightDiagnostic()> emitError,
+                                   ArrayRef<DataLayoutEntryInterface> entries,
+                                   bool allowTypes = true) {
+  DenseSet<DataLayoutEntryKey> keys;
+  for (DataLayoutEntryInterface entry : entries) {
+    if (!entry)
+      return emitError() << "contained invalid DLTI entry";
+    DataLayoutEntryKey key = entry.getKey();
+    if (key.isNull())
+      return emitError() << "contained invalid DLTI key";
+    if (!allowTypes && llvm::dyn_cast<Type>(key))
+      return emitError() << "type as DLIT key is not allowed";
+    if (!keys.insert(key).second)
+      return emitError() << "repeated DLTI key: " << keyToStr(key);
+    if (!entry.getValue())
+      return emitError() << "value associated to DLTI key " << keyToStr(key)
+                         << " is invalid";
+  }
+  return success();
+}
+
 //===----------------------------------------------------------------------===//
 // DataLayoutEntryAttr
 //===----------------------------------------------------------------------===//
@@ -71,15 +189,16 @@ DataLayoutEntryKey DataLayoutEntryAttr::getKey() const {
 Attribute DataLayoutEntryAttr::getValue() const { return getImpl()->value; }
 
 /// Parses an attribute with syntax:
-///   attr ::= `#target.` `dl_entry` `<` (type | quoted-string) `,` attr `>`
-Attribute DataLayoutEntryAttr::parse(AsmParser &parser, Type ty) {
+///   dl-entry-attr ::= `#dlti.` `dl_entry` `<` (type | quoted-string) `,`
+///     attr `>`
+Attribute DataLayoutEntryAttr::parse(AsmParser &parser, Type type) {
   if (failed(parser.parseLess()))
     return {};
 
-  Type type = nullptr;
+  Type typeKey = nullptr;
   std::string identifier;
   SMLoc idLoc = parser.getCurrentLocation();
-  OptionalParseResult parsedType = parser.parseOptionalType(type);
+  OptionalParseResult parsedType = parser.parseOptionalType(typeKey);
   if (parsedType.has_value() && failed(parsedType.value()))
     return {};
   if (!parsedType.has_value()) {
@@ -95,38 +214,29 @@ Attribute DataLayoutEntryAttr::parse(AsmParser &parser, Type ty) {
       failed(parser.parseGreater()))
     return {};
 
-  return type ? get(type, value)
-              : get(parser.getBuilder().getStringAttr(identifier), value);
+  return typeKey ? get(typeKey, value)
+                 : get(parser.getBuilder().getStringAttr(identifier), value);
 }
 
-void DataLayoutEntryAttr::print(AsmPrinter &os) const {
-  os << "<";
-  if (auto type = llvm::dyn_cast_if_present<Type>(getKey()))
-    os << type;
-  else
-    os << "\"" << getKey().get<StringAttr>().strref() << "\"";
-  os << ", " << getValue() << ">";
+void DataLayoutEntryAttr::print(AsmPrinter &printer) const {
+  printer << "<" << keyToStr(getKey()) << ", " << getValue() << ">";
 }
 
 //===----------------------------------------------------------------------===//
 // DLTIMapAttr
 //===----------------------------------------------------------------------===//
 
-static LogicalResult verifyEntries(function_ref<InFlightDiagnostic()> emitError,
-                                   ArrayRef<DataLayoutEntryInterface> entries) {
-  DenseSet<Type> types;
-  DenseSet<StringAttr> ids;
-  for (DataLayoutEntryInterface entry : entries) {
-    if (auto type = llvm::dyn_cast_if_present<Type>(entry.getKey())) {
-      if (!types.insert(type).second)
-        return emitError() << "repeated layout entry key: " << type;
-    } else {
-      auto id = entry.getKey().get<StringAttr>();
-      if (!ids.insert(id).second)
-        return emitError() << "repeated layout entry key: " << id.getValue();
-    }
-  }
-  return success();
+/// Parses an attribute with syntax:
+///   map-attr ::= `#dlti.` `map` `<` entry-list `>`
+///   entry-list ::= entry | entry `,` entry-list
+///   entry ::= ((type | quoted-string) `=` attr) | dl-entry-attr
+Attribute MapAttr::parse(AsmParser &parser, Type type) {
+  return parseAngleBracketedEntries<MapAttr>(parser, type, /*tryType=*/true,
+                                             /*allowEmpty=*/true);
+}
+
+void MapAttr::print(AsmPrinter &printer) const {
+  printAngleBracketedEntries(printer, getEntries());
 }
 
 LogicalResult MapAttr::verify(function_ref<InFlightDiagnostic()> emitError,
@@ -282,98 +392,40 @@ DataLayoutSpecAttr::getStackAlignmentIdentifier(MLIRContext *context) const {
       DLTIDialect::kDataLayoutStackAlignmentKey);
 }
 
-/// Parses an attribute with syntax
-///   attr ::= `#target.` `dl_spec` `<` attr-list? `>`
-///   attr-list ::= attr
-///               | attr `,` attr-list
+/// Parses an attribute with syntax:
+///   dl-spec-attr ::= `#dlti.` `dl_spec` `<` entry-list `>`
+///   entry-list ::= | entry | entry `,` entry-list
+///   entry ::= ((type | quoted-string) = attr) | dl-entry-attr
 Attribute DataLayoutSpecAttr::parse(AsmParser &parser, Type type) {
-  if (failed(parser.parseLess()))
-    return {};
-
-  // Empty spec.
-  if (succeeded(parser.parseOptionalGreater()))
-    return get(parser.getContext(), {});
-
-  SmallVector<DataLayoutEntryInterface> entries;
-  if (parser.parseCommaSeparatedList(
-          [&]() { return parser.parseAttribute(entries.emplace_back()); }) ||
-      parser.parseGreater())
-    return {};
-
-  return getChecked([&] { return parser.emitError(parser.getNameLoc()); },
-                    parser.getContext(), entries);
+  return parseAngleBracketedEntries<DataLayoutSpecAttr>(parser, type,
+                                                        /*tryType=*/true,
+                                                        /*allowEmpty=*/true);
 }
 
-void DataLayoutSpecAttr::print(AsmPrinter &os) const {
-  os << "<";
-  llvm::interleaveComma(getEntries(), os);
-  os << ">";
+void DataLayoutSpecAttr::print(AsmPrinter &printer) const {
+  printAngleBracketedEntries(printer, getEntries());
 }
 
 //===----------------------------------------------------------------------===//
 // TargetDeviceSpecAttr
 //===----------------------------------------------------------------------===//
 
-namespace mlir {
-/// A FieldParser for key-value pairs of DeviceID-target device spec pairs that
-/// make up a target system spec.
-template <>
-struct FieldParser<DeviceIDTargetDeviceSpecPair> {
-  static FailureOr<DeviceIDTargetDeviceSpecPair> parse(AsmParser &parser) {
-    std::string deviceID;
-
-    if (failed(parser.parseString(&deviceID))) {
-      parser.emitError(parser.getCurrentLocation())
-          << "DeviceID is missing, or is not of string type";
-      return failure();
-    }
-
-    if (failed(parser.parseColon())) {
-      parser.emitError(parser.getCurrentLocation()) << "Missing colon";
-      return failure();
-    }
-
-    auto target_device_spec =
-        FieldParser<TargetDeviceSpecInterface>::parse(parser);
-    if (failed(target_device_spec)) {
-      parser.emitError(parser.getCurrentLocation())
-          << "Error in parsing target device spec";
-      return failure();
-    }
-
-    return std::make_pair(parser.getBuilder().getStringAttr(deviceID),
-                          *target_device_spec);
-  }
-};
-
-inline AsmPrinter &operator<<(AsmPrinter &printer,
-                              DeviceIDTargetDeviceSpecPair param) {
-  return printer << param.first << " : " << param.second;
-}
-
-} // namespace mlir
-
 LogicalResult
 TargetDeviceSpecAttr::verify(function_ref<InFlightDiagnostic()> emitError,
                              ArrayRef<DataLayoutEntryInterface> entries) {
-  // Entries in a target device spec can only have StringAttr as key. It does
-  // not support type as a key. Hence not reusing
-  // DataLayoutEntryInterface::verify.
-  DenseSet<StringAttr> ids;
-  for (DataLayoutEntryInterface entry : entries) {
-    if (auto type = llvm::dyn_cast_if_present<Type>(entry.getKey())) {
-      return emitError()
-             << "dlti.target_device_spec does not allow type as a key: "
-             << type;
-    } else {
-      // Check that keys in a target device spec are unique.
-      auto id = entry.getKey().get<StringAttr>();
-      if (!ids.insert(id).second)
-        return emitError() << "repeated layout entry key: " << id.getValue();
-    }
-  }
+  return verifyEntries(emitError, entries, /*allowTypes=*/false);
+}
 
-  return success();
+/// Parses an attribute with syntax:
+///   dev-spec-attr ::= `#dlti.` `target_device_spec` `<` entry-list `>`
+///   entry-list ::= entry | entry `,` entry-list
+///   entry ::= (quoted-string `=` attr) | dl-entry-attr
+Attribute TargetDeviceSpecAttr::parse(AsmParser &parser, Type type) {
+  return parseAngleBracketedEntries<TargetDeviceSpecAttr>(parser, type);
+}
+
+void TargetDeviceSpecAttr::print(AsmPrinter &printer) const {
+  printAngleBracketedEntries(printer, getEntries());
 }
 
 //===----------------------------------------------------------------------===//
@@ -382,27 +434,46 @@ TargetDeviceSpecAttr::verify(function_ref<InFlightDiagnostic()> emitError,
 
 LogicalResult
 TargetSystemSpecAttr::verify(function_ref<InFlightDiagnostic()> emitError,
-                             ArrayRef<DeviceIDTargetDeviceSpecPair> entries) {
-  DenseSet<TargetSystemSpecInterface::DeviceID> device_ids;
+                             ArrayRef<DataLayoutEntryInterface> entries) {
+  DenseSet<TargetSystemSpecInterface::DeviceID> deviceIds;
 
   for (const auto &entry : entries) {
-    TargetDeviceSpecInterface target_device_spec = entry.second;
-
-    // First verify that a target device spec is valid.
-    if (failed(TargetDeviceSpecAttr::verify(emitError,
-                                            target_device_spec.getEntries())))
-      return failure();
+    auto deviceId =
+        llvm::dyn_cast<TargetSystemSpecInterface::DeviceID>(entry.getKey());
+    if (!deviceId)
+      return emitError() << "non-string key of DLTI system spec";
+
+    if (auto targetDeviceSpec =
+            llvm::dyn_cast<TargetDeviceSpecInterface>(entry.getValue())) {
+      if (failed(TargetDeviceSpecAttr::verify(emitError,
+                                              targetDeviceSpec.getEntries())))
+        return failure(); // Assume sub-verifier outputted error message.
+    } else {
+      return emitError() << "value associated with key " << deviceId
+                         << " is not a DLTI device spec";
+    }
 
     // Check that device IDs are unique across all entries.
-    TargetSystemSpecInterface::DeviceID device_id = entry.first;
-    if (!device_ids.insert(device_id).second) {
-      return emitError() << "repeated Device ID in dlti.target_system_spec: "
-                         << device_id;
-    }
+    if (!deviceIds.insert(deviceId).second)
+      return emitError() << "repeated device ID in dlti.target_system_spec: "
+                         << deviceId;
   }
+
   return success();
 }
 
+/// Parses an attribute with syntax:
+///   sys-spec-attr ::= `#dlti.` `target_system_spec` `<` entry-list `>`
+///   entry-list ::= entry | entry `,` entry-list
+///   entry ::= (quoted-string `=` dev-spec-attr) | dl-entry-attr
+Attribute TargetSystemSpecAttr::parse(AsmParser &parser, Type type) {
+  return parseAngleBracketedEntries<TargetSystemSpecAttr>(parser, type);
+}
+
+void TargetSystemSpecAttr::print(AsmPrinter &printer) const {
+  printAngleBracketedEntries(printer, getEntries());
+}
+
 //===----------------------------------------------------------------------===//
 // DLTIDialect
 //===----------------------------------------------------------------------===//
@@ -446,15 +517,6 @@ dlti::query(Operation *op, ArrayRef<DataLayoutEntryKey> keys, bool emitError) {
     return failure();
   }
 
-  auto keyToStr = [](DataLayoutEntryKey key) -> std::string {
-    std::string buf;
-    llvm::TypeSwitch<DataLayoutEntryKey>(key)
-        .Case<StringAttr, Type>( // The only two kinds of key we know of.
-            [&](auto key) { llvm::raw_string_ostream(buf) << key; })
-        .Default([](auto) { llvm_unreachable("unexpected entry key kind"); });
-    return buf;
-  };
-
   Attribute currentAttr = queryable;
   for (auto &&[idx, key] : llvm::enumerate(keys)) {
     if (auto map = llvm::dyn_cast<DLTIQueryInterface>(currentAttr)) {
diff --git a/mlir/lib/Interfaces/DataLayoutInterfaces.cpp b/mlir/lib/Interfaces/DataLayoutInterfaces.cpp
index 2158953c07110a..1c22945c4caaf7 100644
--- a/mlir/lib/Interfaces/DataLayoutInterfaces.cpp
+++ b/mlir/lib/Interfaces/DataLayoutInterfaces.cpp
@@ -790,13 +790,22 @@ mlir::detail::verifyTargetSystemSpec(TargetSystemSpecInterface spec,
   DenseMap<StringAttr, DataLayoutEntryInterface> deviceDescKeys;
   DenseSet<TargetSystemSpecInterface::DeviceID> deviceIDs;
   for (const auto &entry : spec.getEntries()) {
-    TargetDeviceSpecInterface targetDeviceSpec = entry.second;
+    auto targetDeviceSpec =
+        llvm::dyn_cast<TargetDeviceSpecInterface>(entry.getValue());
+
+    if (!targetDeviceSpec)
+      return failure();
+
     // First, verify individual target device desc specs.
     if (failed(targetDeviceSpec.verifyEntry(loc)))
       return failure();
 
     // Check that device IDs are unique across all entries.
-    TargetSystemSpecInterface::DeviceID deviceID = entry.first;
+    auto deviceID =
+        llvm::dyn_cast<TargetSystemSpecInterface::DeviceID>(entry.getKey());
+    if (!deviceID)
+      return failure();
+
     if (!deviceIDs.insert(deviceID).second) {
       return failure();
     }
diff --git a/mlir/test/Dialect/DLTI/invalid.mlir b/mlir/test/Dialect/DLTI/invalid.mlir
index 4b04f0195ef823..ff0b435613fb17 100644
--- a/mlir/test/Dialect/DLTI/invalid.mlir
+++ b/mlir/test/Dialect/DLTI/invalid.mlir
@@ -25,7 +25,7 @@
 
 // -----
 
-// expected-error at below {{repeated layout entry key: test.id}}
+// expected-error at below {{repeated DLTI key: "test.id"}}
 "test.unknown_op"() { test.unknown_attr = #dlti.dl_spec<
   #dlti.dl_entry<"test.id", 42>,
   #dlti.dl_entry<"test.id", 43>
@@ -33,7 +33,7 @@
 
 // -----
 
-// expected-error at below {{repeated layout entry key: 'i32'}}
+// expected-error at below {{repeated DLTI key: i32}}
 "test.unknown_op"() { test.unknown_attr = #dlti.map<
   #dlti.dl_entry<i32, 42>,
   #dlti.dl_entry<i32, 42>
@@ -41,7 +41,7 @@
 
 // -----
 
-// expected-error at below {{repeated layout entry key: 'i32'}}
+// expected-error at below {{repeated DLTI key: i32}}
 "test.unknown_op"() { test.unknown_attr = #dlti.dl_spec<
   #dlti.dl_entry<i32, 42>,
   #dlti.dl_entry<i32, 42>
@@ -111,9 +111,7 @@ module attributes { dlti.dl_spec = #dlti.dl_spec<#dlti.dl_entry<"unknown.unknown
 
 // -----
 
-// expected-error at below {{expected string}}
-// expected-error at below {{DeviceID is missing, or is not of string type}}
-// expected-error at below {{failed to parse DLTI_TargetSystemSpecAttr parameter 'entries' which is to be a `::llvm::ArrayRef<DeviceIDTargetDeviceSpecPair>`}}
+// expected-error at below {{invalid kind of attribute specified}}
 "test.unknown_op"() { dlti.target_system_spec = #dlti.target_system_spec<[]> } : () -> ()
 
 // -----
@@ -121,11 +119,9 @@ module attributes { dlti.dl_spec = #dlti.dl_spec<#dlti.dl_entry<"unknown.unknown
 module attributes {
   // Device ID is missing
   //
-  // expected-error at +4 {{expected string}}
-  // expected-error at +3 {{DeviceID is missing, or is not of string type}}
-  // expected-error at +2 {{failed to parse DLTI_TargetSystemSpecAttr parameter 'entries' which is to be a `::llvm::ArrayRef<DeviceIDTargetDeviceSpecPair>`}}
+  // expected-error at below {{expected attribute value}}
   dlti.target_system_spec = #dlti.target_system_spec<
-    : #dlti.target_device_spec<
+    = #dlti.target_device_spec<
       #dlti.dl_entry<"L1_cache_size_in_bytes", 4096 : i32>>
   >} {}
 
@@ -134,11 +130,9 @@ module attributes {
 module attributes {
   // Device ID is wrong type
   //
-  // expected-error at +4 {{expected string}}
-  // expected-error at +3 {{DeviceID is missing, or is not of string type}}
-  // expected-error at +2 {{failed to parse DLTI_TargetSystemSpecAttr parameter 'entries' which is to be a `::llvm::ArrayRef<DeviceIDTargetDeviceSpecPair>`}}
+  // expected-error at +2 {{invalid kind of attribute specified}}
   dlti.target_system_spec = #dlti.target_system_spec<
-    0: #dlti.target_device_spec<
+    0 = #dlti.target_device_spec<
         #dlti.dl_entry<"L1_cache_size_in_bytes", 4096 : i32>>
   >} {}
 
@@ -147,11 +141,11 @@ module attributes {
 module attributes {
   // Repeated Device ID
   //
-  // expected-error at below {{repeated Device ID in dlti.target_system_spec: "CPU"}}
+  // expected-error at +1 {{repeated device ID in dlti.target_system_spec: "CPU}}
   dlti.target_system_spec = #dlti.target_system_spec<
-    "CPU": #dlti.target_device_spec<
+    "CPU" = #dlti.target_device_spec<
             #dlti.dl_entry<"L1_cache_size_in_bytes", 4096>>,
-    "CPU": #dlti.target_device_spec<
+    "CPU" = #dlti.target_device_spec<
             #dlti.dl_entry<"L1_cache_size_in_bytes", 8192>>
   >} {}
 
@@ -160,11 +154,8 @@ module attributes {
 module attributes {
   // Repeated DLTI entry
   //
-  // expected-error at +4 {{repeated layout entry key: L1_cache_size_in_bytes}}
-  // expected-error at +6 {{Error in parsing target device spec}}
-  // expected-error at +5 {{failed to parse DLTI_TargetSystemSpecAttr parameter 'entries' which is to be a `::llvm::ArrayRef<DeviceIDTargetDeviceSpecPair>`}}
+  // expected-error at +2 {{repeated DLTI key: "L1_cache_size_in_bytes"}}
   dlti.target_system_spec = #dlti.target_system_spec<
-    "CPU": #dlti.target_device_spec<
-            #dlti.dl_entry<"L1_cache_size_in_bytes", 4096>,
-            #dlti.dl_entry<"L1_cache_size_in_bytes", 8192>>
+    "CPU" = #dlti.target_device_spec<"L1_cache_size_in_bytes" = 4096,
+                                     "L1_cache_size_in_bytes" = 8192>
   >} {}
diff --git a/mlir/test/Dialect/DLTI/query.mlir b/mlir/test/Dialect/DLTI/query.mlir
index a793c1a6e8e6a2..3825cee6f16164 100644
--- a/mlir/test/Dialect/DLTI/query.mlir
+++ b/mlir/test/Dialect/DLTI/query.mlir
@@ -1,7 +1,7 @@
 // RUN: mlir-opt -transform-interpreter -canonicalize -split-input-file -verify-diagnostics %s | FileCheck %s
 
-// expected-remark @below {{associated attr 42 : i32}}
-module attributes { test.dlti = #dlti.map<#dlti.dl_entry<"test.id", 42 : i32>>} {
+// expected-remark @below {{attr associated to "test.id" = 42 : i32}}
+module attributes { test.dlti = #dlti.map<"test.id" = 42 : i32> } {
   func.func private @f()
 }
 
@@ -10,7 +10,7 @@ module attributes {transform.with_named_sequence} {
     %funcs = transform.structured.match ops{["func.func"]} in %arg : (!transform.any_op) -> !transform.any_op
     %module = transform.get_parent_op %funcs : (!transform.any_op) -> !transform.any_op
     %param = transform.dlti.query ["test.id"] at %module : (!transform.any_op) -> !transform.any_param
-    transform.debug.emit_param_as_remark %param, "associated attr" at %module : !transform.any_param, !transform.any_op
+    transform.debug.emit_param_as_remark %param, "attr associated to \"test.id\" =" at %module : !transform.any_param, !transform.any_op
     transform.yield
   }
 }
@@ -18,7 +18,7 @@ module attributes {transform.with_named_sequence} {
 // -----
 
 // expected-remark @below {{i32 present in set : unit}}
-module attributes { test.dlti = #dlti.map<#dlti.dl_entry<i32, unit>>} {
+module attributes { test.dlti = #dlti.map<i32 = unit> } {
   func.func private @f()
 }
 
@@ -34,8 +34,8 @@ module attributes {transform.with_named_sequence} {
 
 // -----
 
-// expected-remark @below {{associated attr 32 : i32}}
-module attributes { test.dlti = #dlti.map<#dlti.dl_entry<i32, #dlti.map<#dlti.dl_entry<"width_in_bits", 32 : i32>>>>} {
+// expected-remark @below {{attr associated to i32's "width_in_bits" = 32 : i32}}
+module attributes { test.dlti = #dlti.map<i32 = #dlti.map<"width_in_bits" = 32 : i32>> } {
   func.func private @f()
 }
 
@@ -44,7 +44,7 @@ module attributes {transform.with_named_sequence} {
     %funcs = transform.structured.match ops{["func.func"]} in %arg : (!transform.any_op) -> !transform.any_op
     %module = transform.get_parent_op %funcs : (!transform.any_op) -> !transform.any_op
     %param = transform.dlti.query [i32,"width_in_bits"] at %module : (!transform.any_op) -> !transform.any_param
-    transform.debug.emit_param_as_remark %param, "associated attr" at %module : !transform.any_param, !transform.any_op
+    transform.debug.emit_param_as_remark %param, "attr associated to i32's \"width_in_bits\" =" at %module : !transform.any_param, !transform.any_op
     transform.yield
   }
 }
@@ -53,7 +53,7 @@ module attributes {transform.with_named_sequence} {
 
 // expected-remark @below {{width in bits of i32 = 32 : i64}}
 // expected-remark @below {{width in bits of f64 = 64 : i64}}
-module attributes { test.dlti = #dlti.map<#dlti.dl_entry<"width_in_bits", #dlti.map<#dlti.dl_entry<i32, 32>, #dlti.dl_entry<f64, 64>>>>} {
+module attributes { test.dlti = #dlti.map<"width_in_bits" = #dlti.map<i32 = 32, f64 = 64>> } {
   func.func private @f()
 }
 
@@ -71,8 +71,8 @@ module attributes {transform.with_named_sequence} {
 
 // -----
 
-// expected-remark @below {{associated attr 42 : i32}}
-module attributes { test.dlti = #dlti.dl_spec<#dlti.dl_entry<"test.id", 42 : i32>>} {
+// expected-remark @below {{attr associated to "test.id" = 42 : i32}}
+module attributes { test.dlti = #dlti.dl_spec<"test.id" = 42 : i32> } {
   func.func private @f()
 }
 
@@ -81,32 +81,32 @@ module attributes {transform.with_named_sequence} {
     %funcs = transform.structured.match ops{["func.func"]} in %arg : (!transform.any_op) -> !transform.any_op
     %module = transform.get_parent_op %funcs : (!transform.any_op) -> !transform.any_op
     %param = transform.dlti.query ["test.id"] at %module : (!transform.any_op) -> !transform.any_param
-    transform.debug.emit_param_as_remark %param, "associated attr" at %module : !transform.any_param, !transform.any_op
+    transform.debug.emit_param_as_remark %param, "attr associated to \"test.id\" =" at %module : !transform.any_param, !transform.any_op
     transform.yield
   }
 }
 
 // -----
 
-module attributes { test.dlti = #dlti.dl_spec<#dlti.dl_entry<"test.id", 42 : i32>>} {
-  // expected-remark @below {{associated attr 24 : i32}}
-  func.func private @f() attributes { test.dlti = #dlti.dl_spec<#dlti.dl_entry<"test.id", 24 : i32>>}
+module attributes { test.dlti = #dlti.dl_spec<"test.id" = 42 : i32> } {
+  // expected-remark @below {{attr associated to "test.id" = 24 : i32}}
+  func.func private @f() attributes { test.dlti = #dlti.dl_spec<"test.id" = 24 : i32>}
 }
 
 module attributes {transform.with_named_sequence} {
   transform.named_sequence @__transform_main(%arg: !transform.any_op) {
     %funcs = transform.structured.match ops{["func.func"]} in %arg : (!transform.any_op) -> !transform.any_op
     %param = transform.dlti.query ["test.id"] at %funcs : (!transform.any_op) -> !transform.any_param
-    transform.debug.emit_param_as_remark %param, "associated attr" at %funcs : !transform.any_param, !transform.any_op
+    transform.debug.emit_param_as_remark %param, "attr associated to \"test.id\" =" at %funcs : !transform.any_param, !transform.any_op
     transform.yield
   }
 }
 
 // -----
 
-// expected-remark @below {{associated attr 42 : i32}}
-module attributes { test.dlti = #dlti.dl_spec<#dlti.dl_entry<"test.id", 42 : i32>>} {
-  func.func private @f() attributes { test.dlti = #dlti.dl_spec<#dlti.dl_entry<"test.id", 24 : i32>>}
+// expected-remark @below {{attr associated to "test.id" = 42 : i32}}
+module attributes { test.dlti = #dlti.dl_spec<"test.id" = 42 : i32> } {
+  func.func private @f() attributes { test.dlti = #dlti.dl_spec<"test.id" = 24 : i32> }
 }
 
 module attributes {transform.with_named_sequence} {
@@ -114,14 +114,14 @@ module attributes {transform.with_named_sequence} {
     %funcs = transform.structured.match ops{["func.func"]} in %arg : (!transform.any_op) -> !transform.any_op
     %module = transform.get_parent_op %funcs : (!transform.any_op) -> !transform.any_op
     %param = transform.dlti.query ["test.id"] at %module : (!transform.any_op) -> !transform.any_param
-    transform.debug.emit_param_as_remark %param, "associated attr" at %module : !transform.any_param, !transform.any_op
+    transform.debug.emit_param_as_remark %param, "attr associated to \"test.id\" =" at %module : !transform.any_param, !transform.any_op
     transform.yield
   }
 }
 
 // -----
 
-module attributes { test.dlti = #dlti.dl_spec<#dlti.dl_entry<"test.id", 42 : i32>>} {
+module attributes { test.dlti = #dlti.dl_spec<"test.id" = 42 : i32> } {
   func.func @matmul_tensors(
     %arg0: tensor<?x?xf32>, %arg1: tensor<?x?xf32>, %arg2: tensor<?x?xf32>)
       -> tensor<?x?xf32> {
@@ -144,10 +144,10 @@ module attributes {transform.with_named_sequence} {
 
 // -----
 
-module attributes { test.dlti = #dlti.dl_spec<#dlti.dl_entry<"test.id", 42 : i32>>} {
+module attributes { test.dlti = #dlti.dl_spec<"test.id" = 42 : i32> } {
   func.func @matmul_tensors(
     %arg0: tensor<?x?xf32>, %arg1: tensor<?x?xf32>, %arg2: tensor<?x?xf32>)
-      -> tensor<?x?xf32> attributes {test.dlti = #dlti.dl_spec<#dlti.dl_entry<"test.id", 24 : i32>>} {
+      -> tensor<?x?xf32> attributes {test.dlti = #dlti.dl_spec<"test.id" = 24 : i32> } {
     // expected-remark @below {{associated attr 24 : i32}}
     %0 = linalg.matmul  ins(%arg0, %arg1: tensor<?x?xf32>, tensor<?x?xf32>)
                        outs(%arg2: tensor<?x?xf32>)
@@ -169,8 +169,8 @@ module attributes {transform.with_named_sequence} {
 
 // expected-remark @below {{associated attr 42 : i32}}
 module attributes { test.dlti =
-  #dlti.target_system_spec<"CPU":
-    #dlti.target_device_spec<#dlti.dl_entry<"test.id", 42 : i32>>>} {
+  #dlti.target_system_spec<"CPU" =
+    #dlti.target_device_spec<"test.id" = 42 : i32>> } {
   func.func private @f()
 }
 
@@ -186,8 +186,8 @@ module attributes {transform.with_named_sequence} {
 
 // -----
 
-module attributes { test.dlti = #dlti.target_system_spec<"CPU": #dlti.target_device_spec<#dlti.dl_entry<"test.id", 42 : i32>>,
-                                                         "GPU": #dlti.target_device_spec<#dlti.dl_entry<"test.id", 43 : i32>>>} {
+module attributes { test.dlti = #dlti.target_system_spec<"CPU" = #dlti.target_device_spec<"test.id" = 42 : i32>,
+                                                         "GPU" = #dlti.target_device_spec<"test.id" = 43 : i32>> } {
   // expected-remark @below {{associated attr 43 : i32}}
   func.func private @f()
 }
@@ -203,10 +203,10 @@ module attributes {transform.with_named_sequence} {
 
 // -----
 
-module attributes { test.dlti = #dlti.target_system_spec<"CPU": #dlti.target_device_spec<#dlti.dl_entry<"test.id", 42 : i32>>,
-                                                         "GPU": #dlti.target_device_spec<#dlti.dl_entry<"test.id", 43 : i32>>>} {
+module attributes { test.dlti = #dlti.target_system_spec<"CPU" = #dlti.target_device_spec<"test.id" = 42 : i32>,
+                                                         "GPU" = #dlti.target_device_spec<"test.id" = 43 : i32>> } {
   // expected-remark @below {{associated attr 24 : i32}}
-  func.func private @f() attributes { test.dlti = #dlti.target_system_spec<"CPU": #dlti.target_device_spec<#dlti.dl_entry<"test.id", 24 : i32>>> }
+  func.func private @f() attributes { test.dlti = #dlti.target_system_spec<"CPU" = #dlti.target_device_spec<"test.id" = 24 : i32>> }
 }
 
 module attributes {transform.with_named_sequence} {
@@ -221,9 +221,9 @@ module attributes {transform.with_named_sequence} {
 // -----
 
 module attributes { test.dlti = #dlti.target_system_spec<
-  "CPU": #dlti.target_device_spec<
-    #dlti.dl_entry<"cache::L1::size_in_bytes", 65536 : i32>,
-    #dlti.dl_entry<"cache::L1d::size_in_bytes", 32768 : i32>>> } {
+  "CPU" = #dlti.target_device_spec<
+    "cache::L1::size_in_bytes" = 65536 : i32,
+    "cache::L1d::size_in_bytes" = 32768 : i32>> } {
   // expected-remark @below {{L1::size_in_bytes 65536 : i32}}
   // expected-remark @below {{L1d::size_in_bytes 32768 : i32}}
   func.func private @f()
@@ -242,13 +242,13 @@ module attributes {transform.with_named_sequence} {
 
 // -----
 
-#l1_size = #dlti.map<#dlti.dl_entry<"size_in_bytes", 65536 : i32>>
-#l1d_size = #dlti.map<#dlti.dl_entry<"size_in_bytes", 32768 : i32>>
+#l1_size = #dlti.map<"size_in_bytes" = 65536 : i32>
+#l1d_size = #dlti.map<"size_in_bytes" = 32768 : i32>
 module attributes { test.dlti =
-  #dlti.target_system_spec<"CPU":
-    #dlti.target_device_spec<#dlti.dl_entry<"cache",
-      #dlti.map<#dlti.dl_entry<"L1", #l1_size>,
-                #dlti.dl_entry<"L1d", #l1d_size> >>>> } {
+  #dlti.target_system_spec<"CPU" =
+    #dlti.target_device_spec<"cache" =
+      #dlti.map<"L1" = #l1_size,
+                "L1d" = #l1d_size >>> } {
   // expected-remark @below {{L1::size_in_bytes 65536 : i32}}
   // expected-remark @below {{L1d::size_in_bytes 32768 : i32}}
   func.func private @f()
@@ -268,8 +268,7 @@ module attributes {transform.with_named_sequence} {
 // -----
 
 module attributes { test.dlti = #dlti.target_system_spec<
-  "CPU": #dlti.target_device_spec<
-    #dlti.dl_entry<"inner_most_tile_size", 42 : i32>>>} {
+  "CPU" = #dlti.target_device_spec<"inner_most_tile_size" = 42 : i32>> } {
   // CHECK-LABEL: func @matmul_tensors
   func.func @matmul_tensors(
     %arg0: tensor<?x?xf32>, %arg1: tensor<?x?xf32>, %arg2: tensor<?x?xf32>)
@@ -301,8 +300,8 @@ module attributes {transform.with_named_sequence} {
 
 // expected-note @below {{key "NPU" has no DLTI-mapping per attr: #dlti.target_system_spec}}
 module attributes { test.dlti = #dlti.target_system_spec<
-    "CPU": #dlti.target_device_spec<#dlti.dl_entry<"test.id", 42 : i32>>,
-    "GPU": #dlti.target_device_spec<#dlti.dl_entry<"test.id", 43 : i32>>>} {
+    "CPU" = #dlti.target_device_spec<"test.id" = 42 : i32>,
+    "GPU" = #dlti.target_device_spec<"test.id" = 43 : i32>> } {
   // expected-error @below {{target op of failed DLTI query}}
   func.func private @f()
 }
@@ -320,8 +319,8 @@ module attributes {transform.with_named_sequence} {
 
 // expected-note @below {{key "unspecified" has no DLTI-mapping per attr: #dlti.target_device_spec}}
 module attributes { test.dlti = #dlti.target_system_spec<
-    "CPU": #dlti.target_device_spec<#dlti.dl_entry<"test.id", 42 : i32>>,
-    "GPU": #dlti.target_device_spec<#dlti.dl_entry<"test.id", 43 : i32>>>} {
+    "CPU" = #dlti.target_device_spec<"test.id" = 42 : i32>,
+    "GPU" = #dlti.target_device_spec<"test.id" = 43 : i32>> } {
   // expected-error @below {{target op of failed DLTI query}}
   func.func private @f()
 }
@@ -339,8 +338,8 @@ module attributes {transform.with_named_sequence} {
 
 // expected-note @below {{key "test.id" has no DLTI-mapping per attr: #dlti.target_system_spec}}
 module attributes { test.dlti = #dlti.target_system_spec<
-  "CPU": #dlti.target_device_spec<#dlti.dl_entry<"test.id", 42 : i32>>,
-  "GPU": #dlti.target_device_spec<#dlti.dl_entry<"test.id", 43 : i32>>>} {
+  "CPU" = #dlti.target_device_spec<"test.id" = 42 : i32>,
+  "GPU" = #dlti.target_device_spec<"test.id" = 43 : i32>> } {
   // expected-error @below {{target op of failed DLTI query}}
   func.func private @f()
 }
@@ -357,7 +356,7 @@ module attributes {transform.with_named_sequence} {
 // -----
 
 // expected-note @below {{key "CPU" has no DLTI-mapping per attr: #dlti.dl_spec}}
-module attributes { test.dlti = #dlti.dl_spec<#dlti.dl_entry<"test.id", 42 : i32>>} {
+module attributes { test.dlti = #dlti.dl_spec<"test.id" = 42 : i32> } {
   // expected-error @below {{target op of failed DLTI query}}
   func.func private @f()
 }
@@ -374,7 +373,7 @@ module attributes {transform.with_named_sequence} {
 // -----
 
 // expected-note @below {{got non-DLTI-queryable attribute upon looking up keys ["CPU"]}}
-module attributes { test.dlti = #dlti.dl_spec<#dlti.dl_entry<"CPU", 42 : i32>>} {
+module attributes { test.dlti = #dlti.dl_spec<"CPU" = 42 : i32> } {
   // expected-error @below {{target op of failed DLTI query}}
   func.func private @f()
 }
@@ -391,7 +390,7 @@ module attributes {transform.with_named_sequence} {
 // -----
 
 // expected-note @below {{got non-DLTI-queryable attribute upon looking up keys [i32]}}
-module attributes { test.dlti = #dlti.dl_spec<#dlti.dl_entry<i32, 32 : i32>>} {
+module attributes { test.dlti = #dlti.dl_spec<i32 = 32 : i32> } {
   // expected-error @below {{target op of failed DLTI query}}
   func.func private @f()
 }
@@ -424,8 +423,8 @@ module attributes {transform.with_named_sequence} {
 
 // -----
 
-// expected-note @below {{key i64 has no DLTI-mapping per attr: #dlti.map<#dlti.dl_entry<i32, 32 : i64>>}}
-module attributes { test.dlti = #dlti.map<#dlti.dl_entry<"width_in_bits", #dlti.map<#dlti.dl_entry<i32, 32>>>>} {
+// expected-note @below {{key i64 has no DLTI-mapping per attr: #dlti.map<i32 = 32 : i64>}}
+module attributes { test.dlti = #dlti.map<"width_in_bits" = #dlti.map<i32 = 32>>} {
   // expected-error @below {{target op of failed DLTI query}}
   func.func private @f()
 }
@@ -441,7 +440,7 @@ module attributes {transform.with_named_sequence} {
 
 // -----
 
-module attributes { test.dlti = #dlti.dl_spec<#dlti.dl_entry<"test.id", 42 : i32>>} {
+module attributes { test.dlti = #dlti.dl_spec<"test.id" = 42 : i32>} {
   func.func private @f()
 }
 
diff --git a/mlir/test/Dialect/DLTI/roundtrip.mlir b/mlir/test/Dialect/DLTI/roundtrip.mlir
index 43188aad595a7a..361c8bee78e1e4 100644
--- a/mlir/test/Dialect/DLTI/roundtrip.mlir
+++ b/mlir/test/Dialect/DLTI/roundtrip.mlir
@@ -15,13 +15,21 @@
   test.unknown_attr_4 = #dlti.dl_entry<memref<?x?xf32>, ["string", 10]>,
   // CHECK: #dlti.dl_spec<>
   test.unknown_attr_5 = #dlti.dl_spec<>,
-  // CHECK: #dlti.dl_spec<#dlti.dl_entry<"test.id", 42 : i32>>
-  test.unknown_attr_6 = #dlti.dl_spec<#dlti.dl_entry<"test.id", 42 : i32>>,
+  // CHECK: #dlti.dl_spec<"test.id" = 42 : i32>
+  test.unknown_attr_6 = #dlti.dl_spec<"test.id" = 42 : i32>,
   // CHECK: #dlti.dl_spec<
-  // CHECK:   #dlti.dl_entry<"test.id1", 43 : index>
-  // CHECK:   #dlti.dl_entry<"test.id2", 44 : index>
-  // CHECK:   #dlti.dl_entry<"test.id3", 45 : index>>
+  // CHECK:   "test.id1" = 43 : index,
+  // CHECK:   "test.id2" = 44 : index,
+  // CHECK:   "test.id3" = 45 : index>
   test.unknown_attr_7 = #dlti.dl_spec<
+    "test.id1" = 43 : index,
+    "test.id2" = 44 : index,
+    "test.id3" = 45 : index>,
+  // CHECK: #dlti.dl_spec<
+  // CHECK:   "test.id1" = 43 : index,
+  // CHECK:   "test.id2" = 44 : index,
+  // CHECK:   "test.id3" = 45 : index>
+  test.unknown_attr_7_unsugared = #dlti.dl_spec<
     #dlti.dl_entry<"test.id1", 43 : index>,
     #dlti.dl_entry<"test.id2", 44 : index>,
     #dlti.dl_entry<"test.id3", 45 : index>>
@@ -40,34 +48,34 @@
 
 // Should not fail on nested compatible layouts.
 "test.op_with_data_layout"() ({
-  "test.op_with_data_layout"() { dlti.dl_spec = #dlti.dl_spec<#dlti.dl_entry<"unknown.unknown", 32>> } : () -> ()
+  "test.op_with_data_layout"() { dlti.dl_spec = #dlti.dl_spec<"unknown.unknown" = 32> } : () -> ()
   "test.maybe_terminator_op"() : () -> ()
-}) { dlti.dl_spec = #dlti.dl_spec<#dlti.dl_entry<"unknown.unknown", 32>> } : () -> ()
+}) { dlti.dl_spec = #dlti.dl_spec<"unknown.unknown" = 32> } : () -> ()
 
 // Should not fail on deeper nested compatible layouts.
 "test.op_with_data_layout"() ({
   "test.op_with_data_layout"() ({
     "test.op_with_data_layout"()
-       { dlti.dl_spec = #dlti.dl_spec<#dlti.dl_entry<"unknown.unknown", 32>> } : () -> ()
+       { dlti.dl_spec = #dlti.dl_spec<"unknown.unknown" = 32> } : () -> ()
     "test.maybe_terminator_op"() : () -> ()
-  }) { dlti.dl_spec = #dlti.dl_spec<#dlti.dl_entry<"unknown.unknown", 32>> } : () -> ()
+  }) { dlti.dl_spec = #dlti.dl_spec<"unknown.unknown" = 32> } : () -> ()
   "test.maybe_terminator_op"() : () -> ()
-}) { dlti.dl_spec = #dlti.dl_spec<#dlti.dl_entry<"unknown.unknown", 32>> } : () -> ()
+}) { dlti.dl_spec = #dlti.dl_spec<"unknown.unknown" = 32> } : () -> ()
 
 // A valid target system description
 // CHECK: module attributes {
-// CHECK: dlti.target_system_spec = #dlti.target_system_spec<
-// CHECK:  "CPU" : #dlti.target_device_spec<
-// CHECK:   #dlti.dl_entry<"dlti.L1_cache_size_in_bytes", 4096 : ui32>>,
-// CHECK: "GPU" : #dlti.target_device_spec<
-// CHECK:   #dlti.dl_entry<"dlti.max_vector_op_width", 128 : ui32>>
-// CHECK: >} {
+// CHECK:   dlti.target_system_spec = #dlti.target_system_spec<
+// CHECK:     "CPU" = #dlti.target_device_spec<
+// CHECK:       "dlti.L1_cache_size_in_bytes" = 4096 : ui32>,
+// CHECK:    "GPU" = #dlti.target_device_spec<
+// CHECK:       "dlti.max_vector_op_width" = 128 : ui32>
+// CHECK:   >} {
 // CHECK: }
 module attributes {
   dlti.target_system_spec = #dlti.target_system_spec<
-    "CPU": #dlti.target_device_spec<
-      #dlti.dl_entry<"dlti.L1_cache_size_in_bytes", 4096 : ui32>>,
-    "GPU": #dlti.target_device_spec<
-      #dlti.dl_entry<"dlti.max_vector_op_width", 128 : ui32>>
+    "CPU" = #dlti.target_device_spec<
+      "dlti.L1_cache_size_in_bytes" = 4096 : ui32>,
+    "GPU" = #dlti.target_device_spec<
+      "dlti.max_vector_op_width" = 128 : ui32>
   >} {}
- 
+
diff --git a/mlir/test/Dialect/DLTI/valid.mlir b/mlir/test/Dialect/DLTI/valid.mlir
index 31c925e5cb5bed..4c24e80041003b 100644
--- a/mlir/test/Dialect/DLTI/valid.mlir
+++ b/mlir/test/Dialect/DLTI/valid.mlir
@@ -3,12 +3,28 @@
 
 // CHECK:      module attributes {
 // CHECK-SAME:   dlti.map = #dlti.map<
-// CHECK-SAME:     #dlti.dl_entry<"magic_num", 42 : i32>,
-// CHECK-SAME:     #dlti.dl_entry<"magic_num_float", 4.242000e+01 : f32>,
-// CHECK-SAME:     #dlti.dl_entry<"magic_type", i32>,
-// CHECK-SAME:     #dlti.dl_entry<i32,
-// CHECK-SAME:       #dlti.map<#dlti.dl_entry<"bitwidth", 32 : i32>>>
-// CHECK-SAME:   >} {
+// CHECK-SAME:     "magic_num" = 42 : i32,
+// CHECK-SAME:     "magic_num_float" = 4.242000e+01 : f32,
+// CHECK-SAME:     "magic_type" = i32,
+// CHECK-SAME:     i32 = #dlti.map<"bitwidth" = 32 : i32>
+// CHECK:        >} {
+// CHECK:      }
+module attributes {
+  dlti.map = #dlti.map<"magic_num" = 42 : i32,
+                       "magic_num_float" = 42.42 : f32,
+                       "magic_type" = i32,
+                        i32 = #dlti.map<"bitwidth" = 32 : i32>>
+  } {}
+
+// -----
+
+// CHECK:      module attributes {
+// CHECK-SAME:   dlti.map = #dlti.map<
+// CHECK-SAME:     "magic_num" = 42 : i32,
+// CHECK-SAME:     "magic_num_float" = 4.242000e+01 : f32,
+// CHECK-SAME:     "magic_type" = i32,
+// CHECK-SAME:     i32 = #dlti.map<"bitwidth" = 32 : i32>
+// CHECK:        >} {
 // CHECK:      }
 module attributes {
   dlti.map = #dlti.map<
@@ -21,13 +37,11 @@ module attributes {
 // -----
 
 // CHECK:      module attributes {
-// CHECK-SAME:  dlti.map = #dlti.map<
-// CHECK-SAME:    #dlti.dl_entry<"CPU", #dlti.map<
-// CHECK-SAME:      #dlti.dl_entry<"L1_cache_size_in_bytes", 4096 : i32>>>,
-// CHECK-SAME:    #dlti.dl_entry<"GPU", #dlti.map<
-// CHECK-SAME:      #dlti.dl_entry<"max_vector_op_width", 128 : i32>>>
-// CHECK-SAME:  >} {
-// CHECK:        }
+// CHECK-SAME:   dlti.map = #dlti.map<
+// CHECK-SAME:     "CPU" = #dlti.map<"L1_cache_size_in_bytes" = 4096 : i32>,
+// CHECK-SAME:     "GPU" = #dlti.map<"max_vector_op_width" = 128 : i32>
+// CHECK-SAME:   >} {
+// CHECK:      }
 module attributes {
   dlti.map = #dlti.map<
     #dlti.dl_entry<"CPU", #dlti.map<
@@ -40,17 +54,17 @@ module attributes {
 
 // CHECK:      module attributes {
 // CHECK-SAME:  dlti.target_system_spec = #dlti.target_system_spec<
-// CHECK-SAME:    "CPU" : #dlti.target_device_spec<
-// CHECK-SAME:      #dlti.dl_entry<"L1_cache_size_in_bytes", 4096 : i32>>,
-// CHECK-SAME:    "GPU" : #dlti.target_device_spec<
-// CHECK-SAME:      #dlti.dl_entry<"max_vector_op_width", 128 : i32>>
+// CHECK-SAME:    "CPU" = #dlti.target_device_spec<
+// CHECK-SAME:      "L1_cache_size_in_bytes" = 4096 : i32>,
+// CHECK-SAME:    "GPU" = #dlti.target_device_spec<
+// CHECK-SAME:      "max_vector_op_width" = 128 : i32>
 // CHECK-SAME:  >} {
-// CHECK:        }
+// CHECK:      }
 module attributes {
   dlti.target_system_spec = #dlti.target_system_spec<
-    "CPU": #dlti.target_device_spec<
+    "CPU" = #dlti.target_device_spec<
       #dlti.dl_entry<"L1_cache_size_in_bytes", 4096 : i32>>,
-    "GPU": #dlti.target_device_spec<
+    "GPU" = #dlti.target_device_spec<
       #dlti.dl_entry<"max_vector_op_width", 128 : i32>>
   >} {}
 
@@ -58,89 +72,89 @@ module attributes {
 
 // CHECK:      module attributes {
 // CHECK-SAME:  dlti.target_system_spec = #dlti.target_system_spec<
-// CHECK-SAME:    "CPU" : #dlti.target_device_spec<
-// CHECK-SAME:      #dlti.dl_entry<"L1_cache_size_in_bytes", 4096 : i32>>,
-// CHECK-SAME:    "GPU" : #dlti.target_device_spec<
-// CHECK-SAME:      #dlti.dl_entry<"L1_cache_size_in_bytes", 8192 : i32>>
+// CHECK-SAME:    "CPU" = #dlti.target_device_spec<
+// CHECK-SAME:      "L1_cache_size_in_bytes" = 4096 : i32>,
+// CHECK-SAME:    "GPU" = #dlti.target_device_spec<
+// CHECK-SAME:      "L1_cache_size_in_bytes" = 8192 : i32>
 // CHECK-SAME:  >} {
-// CHECK:        }
+// CHECK:      }
 module attributes {
   dlti.target_system_spec = #dlti.target_system_spec<
-    "CPU": #dlti.target_device_spec<
+    "CPU" = #dlti.target_device_spec<
       #dlti.dl_entry<"L1_cache_size_in_bytes", 4096 : i32>>,
-    "GPU": #dlti.target_device_spec<
+    "GPU" = #dlti.target_device_spec<
       #dlti.dl_entry<"L1_cache_size_in_bytes", 8192 : i32>>
   >} {}
 
 // -----
 
 // CHECK:      module attributes {
-// CHECK-SAME:  dlti.target_system_spec = #dlti.target_system_spec<
-// CHECK-SAME:    "CPU" : #dlti.target_device_spec<
-// CHECK-SAME:      #dlti.dl_entry<"L1_cache_size_in_bytes", 4096 : i64>>,
-// CHECK-SAME:    "GPU" : #dlti.target_device_spec<
-// CHECK-SAME:      #dlti.dl_entry<"L1_cache_size_in_bytes", 8192 : i64>>
-// CHECK-SAME:  >} {
-// CHECK:        }
+// CHECK-SAME:   dlti.target_system_spec = #dlti.target_system_spec<
+// CHECK-SAME:     "CPU" = #dlti.target_device_spec<
+// CHECK-SAME:       "L1_cache_size_in_bytes" = 4096 : i64>,
+// CHECK-SAME:     "GPU" = #dlti.target_device_spec<
+// CHECK-SAME:       "L1_cache_size_in_bytes" = 8192 : i64>
+// CHECK-SAME:   >} {
+// CHECK:      }
 module attributes {
   dlti.target_system_spec = #dlti.target_system_spec<
-    "CPU": #dlti.target_device_spec<
+    "CPU" = #dlti.target_device_spec<
       #dlti.dl_entry<"L1_cache_size_in_bytes", 4096 : i64>>,
-    "GPU": #dlti.target_device_spec<
+    "GPU" = #dlti.target_device_spec<
       #dlti.dl_entry<"L1_cache_size_in_bytes", 8192 : i64>>
   >} {}
 
 // -----
 
 // CHECK:      module attributes {
-// CHECK-SAME:  dlti.target_system_spec = #dlti.target_system_spec<
-// CHECK-SAME:    "CPU" : #dlti.target_device_spec<
-// CHECK-SAME:      #dlti.dl_entry<"max_vector_op_width", 64 : i32>>,
-// CHECK-SAME:    "GPU" : #dlti.target_device_spec<
-// CHECK-SAME:      #dlti.dl_entry<"max_vector_op_width", 128 : i32>>
-// CHECK-SAME:  >} {
-// CHECK:        }
+// CHECK-SAME:   dlti.target_system_spec = #dlti.target_system_spec<
+// CHECK-SAME:     "CPU" = #dlti.target_device_spec<
+// CHECK-SAME:       "max_vector_op_width" = 64 : i32>,
+// CHECK-SAME:     "GPU" = #dlti.target_device_spec<
+// CHECK-SAME:       "max_vector_op_width" = 128 : i32>
+// CHECK-SAME:   >} {
+// CHECK:      }
 module attributes {
   dlti.target_system_spec = #dlti.target_system_spec<
-    "CPU": #dlti.target_device_spec<
+    "CPU" = #dlti.target_device_spec<
       #dlti.dl_entry<"max_vector_op_width", 64 : i32>>,
-    "GPU": #dlti.target_device_spec<
+    "GPU" = #dlti.target_device_spec<
       #dlti.dl_entry<"max_vector_op_width", 128 : i32>>
   >} {}
 
 // -----
 
 // CHECK:      module attributes {
-// CHECK-SAME:  dlti.target_system_spec = #dlti.target_system_spec<
-// CHECK-SAME:    "CPU" : #dlti.target_device_spec<
-// CHECK-SAME:      #dlti.dl_entry<"max_vector_op_width", 64 : i64>>,
-// CHECK-SAME:    "GPU" : #dlti.target_device_spec<
-// CHECK-SAME:      #dlti.dl_entry<"max_vector_op_width", 128 : i64>>
-// CHECK-SAME:  >} {
-// CHECK:        }
+// CHECK-SAME:   dlti.target_system_spec = #dlti.target_system_spec<
+// CHECK-SAME:     "CPU" = #dlti.target_device_spec<
+// CHECK-SAME:       "max_vector_op_width" = 64 : i64>,
+// CHECK-SAME:     "GPU" = #dlti.target_device_spec<
+// CHECK-SAME:       "max_vector_op_width" = 128 : i64>
+// CHECK-SAME:   >} {
+// CHECK:      }
 module attributes {
   dlti.target_system_spec = #dlti.target_system_spec<
-    "CPU": #dlti.target_device_spec<
+    "CPU" = #dlti.target_device_spec<
       #dlti.dl_entry<"max_vector_op_width", 64 : i64>>,
-    "GPU": #dlti.target_device_spec<
+    "GPU" = #dlti.target_device_spec<
       #dlti.dl_entry<"max_vector_op_width", 128 : i64>>
   >} {}
 
 // -----
 
 // CHECK:      module attributes {
-// CHECK-SAME:  dlti.target_system_spec = #dlti.target_system_spec<
-// CHECK-SAME:    "CPU" : #dlti.target_device_spec<
-// CHECK-SAME:      #dlti.dl_entry<"max_vector_op_width", 64 : i64>>,
-// CHECK-SAME:    "GPU" : #dlti.target_device_spec<
-// CHECK-SAME:      #dlti.dl_entry<"max_vector_op_width", 128 : i64>>
-// CHECK-SAME:  >} {
-// CHECK:        }
+// CHECK-SAME:   dlti.target_system_spec = #dlti.target_system_spec<
+// CHECK-SAME:     "CPU" = #dlti.target_device_spec<
+// CHECK-SAME:       "max_vector_op_width" = 64 : i64>,
+// CHECK-SAME:     "GPU" = #dlti.target_device_spec<
+// CHECK-SAME:       "max_vector_op_width" = 128 : i64>
+// CHECK-SAME:   >} {
+// CHECK:      }
 module attributes {
   dlti.target_system_spec = #dlti.target_system_spec<
-    "CPU": #dlti.target_device_spec<
+    "CPU" = #dlti.target_device_spec<
       #dlti.dl_entry<"max_vector_op_width", 64 : i64>>,
-    "GPU": #dlti.target_device_spec<
+    "GPU" = #dlti.target_device_spec<
       #dlti.dl_entry<"max_vector_op_width", 128 : i64>>
   >} {}
 
@@ -149,18 +163,18 @@ module attributes {
 // Check values of mixed type
 //
 // CHECK:      module attributes {
-// CHECK-SAME:  dlti.target_system_spec = #dlti.target_system_spec<
-// CHECK-SAME:    "CPU" : #dlti.target_device_spec<
-// CHECK-SAME:      #dlti.dl_entry<"L1_cache_size_in_bytes", 4096 : ui32>>,
-// CHECK-SAME:    "GPU" : #dlti.target_device_spec<
-// CHECK-SAME:      #dlti.dl_entry<"max_vector_op_width", "128">>
-// CHECK-SAME:  >} {
-// CHECK:        }
+// CHECK-SAME:   dlti.target_system_spec = #dlti.target_system_spec<
+// CHECK-SAME:     "CPU" = #dlti.target_device_spec<
+// CHECK-SAME:       "L1_cache_size_in_bytes" = 4096 : ui32>,
+// CHECK-SAME:     "GPU" = #dlti.target_device_spec<
+// CHECK-SAME:       "max_vector_op_width" = "128">
+// CHECK-SAME:   >} {
+// CHECK:      }
 module attributes {
   dlti.target_system_spec = #dlti.target_system_spec<
-    "CPU": #dlti.target_device_spec<
+    "CPU" = #dlti.target_device_spec<
       #dlti.dl_entry<"L1_cache_size_in_bytes", 4096 : ui32>>,
-    "GPU": #dlti.target_device_spec<
+    "GPU" = #dlti.target_device_spec<
       #dlti.dl_entry<"max_vector_op_width", "128">>
   >} {}
 
@@ -169,18 +183,18 @@ module attributes {
 // Check values of mixed type
 //
 // CHECK:      module attributes {
-// CHECK-SAME:  dlti.target_system_spec = #dlti.target_system_spec<
-// CHECK-SAME:    "CPU" : #dlti.target_device_spec<
-// CHECK-SAME:      #dlti.dl_entry<"max_vector_op_width", 4.096000e+03 : f32>>,
-// CHECK-SAME:    "GPU" : #dlti.target_device_spec<
-// CHECK-SAME:      #dlti.dl_entry<"L1_cache_size_in_bytes", "128">>
-// CHECK-SAME:  >} {
-// CHECK:        }
+// CHECK-SAME:   dlti.target_system_spec = #dlti.target_system_spec<
+// CHECK-SAME:     "CPU" = #dlti.target_device_spec<
+// CHECK-SAME:       "max_vector_op_width" = 4.096000e+03 : f32>,
+// CHECK-SAME:     "GPU" = #dlti.target_device_spec<
+// CHECK-SAME:       "L1_cache_size_in_bytes" = "128">
+// CHECK-SAME:   >} {
+// CHECK:      }
 module attributes {
   dlti.target_system_spec = #dlti.target_system_spec<
-    "CPU": #dlti.target_device_spec<
+    "CPU" = #dlti.target_device_spec<
       #dlti.dl_entry<"max_vector_op_width", 4096.0 : f32>>,
-    "GPU": #dlti.target_device_spec<
+    "GPU" = #dlti.target_device_spec<
       #dlti.dl_entry<"L1_cache_size_in_bytes", "128">>
   >} {}
 
@@ -190,34 +204,51 @@ module attributes {
 // Check values of mixed type
 //
 // CHECK:      module attributes {
-// CHECK-SAME:  dlti.target_system_spec = #dlti.target_system_spec<
-// CHECK-SAME:    "CPU" : #dlti.target_device_spec<
-// CHECK-SAME:      #dlti.dl_entry<"vector_unit", #dlti.map<
-// CHECK-SAME:        #dlti.dl_entry<"max_op_width", 4.096000e+03 : f32>>>>,
-// CHECK-SAME:    "GPU" : #dlti.target_device_spec<
-// CHECK-SAME:      #dlti.dl_entry<"L1_cache_size_in_bytes", "128">>
-// CHECK-SAME:  >} {
-// CHECK:        }
+// CHECK-SAME:   dlti.target_system_spec = #dlti.target_system_spec<
+// CHECK-SAME:     "CPU" = #dlti.target_device_spec<
+// CHECK-SAME:       "vector_unit" = #dlti.map<
+// CHECK-SAME:         "max_op_width" = 4.096000e+03 : f32>>,
+// CHECK-SAME:     "GPU" = #dlti.target_device_spec<
+// CHECK-SAME:       "L1_cache_size_in_bytes" = "128">
+// CHECK-SAME:   >} {
+// CHECK:      }
 module attributes {
   dlti.target_system_spec = #dlti.target_system_spec<
-    "CPU": #dlti.target_device_spec<
+    "CPU" = #dlti.target_device_spec<
       #dlti.dl_entry<"vector_unit", #dlti.map<
         #dlti.dl_entry<"max_op_width", 4096.0 : f32>>>>,
-    "GPU": #dlti.target_device_spec<
+    "GPU" = #dlti.target_device_spec<
       #dlti.dl_entry<"L1_cache_size_in_bytes", "128">>
   >} {}
 
+// -----
+
+// Check values of mixed type
+//
+// CHECK:      module attributes {
+// CHECK-SAME:   dlti.target_system_spec = #dlti.target_system_spec<
+// CHECK-SAME:     "CPU" = #dlti.target_device_spec<
+// CHECK-SAME:       "L1_cache_size_in_bytes" = 4096 : ui32>,
+// CHECK-SAME:     "GPU" = #dlti.target_device_spec<
+// CHECK-SAME:       "max_vector_op_width" = 128 : i64>
+// CHECK-SAME:   >} {
+// CHECK:      }
+module attributes {
+  dlti.target_system_spec = #dlti.target_system_spec<
+    "CPU" = #dlti.target_device_spec<"L1_cache_size_in_bytes" = 4096 : ui32>,
+    "GPU" = #dlti.target_device_spec<"max_vector_op_width" = 128>
+  >} {}
 
 // -----
 
 // CHECK: "test.op_with_dlti_map"() ({
-// CHECK: }) {dlti.map = #dlti.map<#dlti.dl_entry<"dlti.unknown_id", 42 : i64>>}
+// CHECK: }) {dlti.map = #dlti.map<"dlti.unknown_id" = 42 : i64>}
 "test.op_with_dlti_map"() ({
 }) { dlti.map = #dlti.map<#dlti.dl_entry<"dlti.unknown_id", 42>> } : () -> ()
 
 // -----
 
 // CHECK: "test.op_with_dlti_map"() ({
-// CHECK: }) {dlti.map = #dlti.map<#dlti.dl_entry<i32, 42 : i64>>}
+// CHECK: }) {dlti.map = #dlti.map<i32 = 42 : i64>}
 "test.op_with_dlti_map"() ({
 }) { dlti.map = #dlti.map<#dlti.dl_entry<i32, 42>> } : () -> ()
diff --git a/mlir/test/Dialect/GPU/outlining.mlir b/mlir/test/Dialect/GPU/outlining.mlir
index 7f44f11b47e064..6e682b26f6c95c 100644
--- a/mlir/test/Dialect/GPU/outlining.mlir
+++ b/mlir/test/Dialect/GPU/outlining.mlir
@@ -36,7 +36,7 @@ func.func @launch() {
   return
 }
 
-// CHECK-DL-LABEL: gpu.module @launch_kernel attributes {dlti.dl_spec = #dlti.dl_spec<#dlti.dl_entry<index, 32 : i32>>}
+// CHECK-DL-LABEL: gpu.module @launch_kernel attributes {dlti.dl_spec = #dlti.dl_spec<index = 32 : i32>}
 // CHECK-LABEL: gpu.module @launch_kernel
 // CHECK-NEXT: gpu.func @launch_kernel
 // CHECK-SAME: (%[[KERNEL_ARG0:.*]]: f32, %[[KERNEL_ARG1:.*]]: memref<?xf32, 1>)
@@ -123,7 +123,7 @@ llvm.func @launch_from_llvm_func() {
   llvm.return
 }
 
-// CHECK-DL-LABEL: gpu.module @launch_from_llvm_func_kernel attributes {dlti.dl_spec = #dlti.dl_spec<#dlti.dl_entry<index, 32 : i32>>}
+// CHECK-DL-LABEL: gpu.module @launch_from_llvm_func_kernel attributes {dlti.dl_spec = #dlti.dl_spec<index = 32 : i32>}
 
 // -----
 
@@ -169,8 +169,8 @@ func.func @multiple_launches() {
   return
 }
 
-// CHECK-DL-LABEL: gpu.module @multiple_launches_kernel attributes {dlti.dl_spec = #dlti.dl_spec<#dlti.dl_entry<index, 32 : i32>>}
-// CHECK-DL-LABEL: gpu.module @multiple_launches_kernel_0 attributes {dlti.dl_spec = #dlti.dl_spec<#dlti.dl_entry<index, 32 : i32>>}
+// CHECK-DL-LABEL: gpu.module @multiple_launches_kernel attributes {dlti.dl_spec = #dlti.dl_spec<index = 32 : i32>}
+// CHECK-DL-LABEL: gpu.module @multiple_launches_kernel_0 attributes {dlti.dl_spec = #dlti.dl_spec<index = 32 : i32>}
 
 // CHECK: gpu.module @multiple_launches_kernel
 // CHECK: func @multiple_launches_kernel
@@ -197,7 +197,7 @@ func.func @extra_constants_not_inlined(%arg0: memref<?xf32>) {
   return
 }
 
-// CHECK-DL-LABEL: gpu.module @extra_constants_not_inlined_kernel attributes {dlti.dl_spec = #dlti.dl_spec<#dlti.dl_entry<index, 32 : i32>>}
+// CHECK-DL-LABEL: gpu.module @extra_constants_not_inlined_kernel attributes {dlti.dl_spec = #dlti.dl_spec<index = 32 : i32>}
 
 // CHECK-LABEL: func @extra_constants_not_inlined_kernel(%{{.*}}: memref<?xf32>, %{{.*}}: index)
 // CHECK: arith.constant 2
@@ -223,7 +223,7 @@ func.func @extra_constants(%arg0: memref<?xf32>) {
   return
 }
 
-// CHECK-DL-LABEL: gpu.module @extra_constants_kernel attributes {dlti.dl_spec = #dlti.dl_spec<#dlti.dl_entry<index, 32 : i32>>}
+// CHECK-DL-LABEL: gpu.module @extra_constants_kernel attributes {dlti.dl_spec = #dlti.dl_spec<index = 32 : i32>}
 
 // CHECK-LABEL: func @extra_constants_kernel(
 // CHECK-SAME: %[[KARG0:.*]]: memref<?xf32>
@@ -253,7 +253,7 @@ func.func @extra_constants_noarg(%arg0: memref<?xf32>, %arg1: memref<?xf32>) {
   return
 }
 
-// CHECK-DL-LABEL: gpu.module @extra_constants_noarg_kernel attributes {dlti.dl_spec = #dlti.dl_spec<#dlti.dl_entry<index, 32 : i32>>}
+// CHECK-DL-LABEL: gpu.module @extra_constants_noarg_kernel attributes {dlti.dl_spec = #dlti.dl_spec<index = 32 : i32>}
 
 // CHECK-LABEL: func @extra_constants_noarg_kernel(
 // CHECK-SAME: %[[KARG0:.*]]: memref<?xf32>, %[[KARG1:.*]]: index
@@ -283,7 +283,7 @@ func.func @multiple_uses(%arg0 : memref<?xf32>) {
   return
 }
 
-// CHECK-DL-LABEL: gpu.module @multiple_uses_kernel attributes {dlti.dl_spec = #dlti.dl_spec<#dlti.dl_entry<index, 32 : i32>>}
+// CHECK-DL-LABEL: gpu.module @multiple_uses_kernel attributes {dlti.dl_spec = #dlti.dl_spec<index = 32 : i32>}
 
 // -----
 
@@ -312,7 +312,7 @@ func.func @multiple_uses2(%arg0 : memref<*xf32>) {
   return
 }
 
-// CHECK-DL-LABEL: gpu.module @multiple_uses2_kernel attributes {dlti.dl_spec = #dlti.dl_spec<#dlti.dl_entry<index, 32 : i32>>}
+// CHECK-DL-LABEL: gpu.module @multiple_uses2_kernel attributes {dlti.dl_spec = #dlti.dl_spec<index = 32 : i32>}
 
 // -----
 
@@ -343,7 +343,7 @@ func.func @recursive_device_function() {
   return
 }
 
-// CHECK-DL-LABEL: gpu.module @function_call_kernel attributes {dlti.dl_spec = #dlti.dl_spec<#dlti.dl_entry<index, 32 : i32>>}
+// CHECK-DL-LABEL: gpu.module @function_call_kernel attributes {dlti.dl_spec = #dlti.dl_spec<index = 32 : i32>}
 
 // CHECK: gpu.module @function_call_kernel {
 // CHECK:   gpu.func @function_call_kernel()
@@ -373,7 +373,7 @@ func.func @non_constant_launches(%arg0 : index) {
   return
 }
 
-// CHECK-DL-LABEL: gpu.module @non_constant_launches_kernel attributes {dlti.dl_spec = #dlti.dl_spec<#dlti.dl_entry<index, 32 : i32>>}
+// CHECK-DL-LABEL: gpu.module @non_constant_launches_kernel attributes {dlti.dl_spec = #dlti.dl_spec<index = 32 : i32>}
 
 // CHECK: module attributes {gpu.container_module}
 
@@ -401,7 +401,7 @@ func.func @launch_memory_attributions_0() {
   return
 }
 
-// CHECK-DL-LABEL: gpu.module @launch_memory_attributions_0_kernel attributes {dlti.dl_spec = #dlti.dl_spec<#dlti.dl_entry<index, 32 : i32>>}
+// CHECK-DL-LABEL: gpu.module @launch_memory_attributions_0_kernel attributes {dlti.dl_spec = #dlti.dl_spec<index = 32 : i32>}
 
 // CHECK-LABEL: gpu.module @launch_memory_attributions_0_kernel
 // CHECK-NEXT: gpu.func @launch_memory_attributions_0_kernel
@@ -435,7 +435,7 @@ func.func @launch_memory_attributions_1(%arg0 : memref<*xf32>) {
   return
 }
 
-// CHECK-DL-LABEL: gpu.module @launch_memory_attributions_1_kernel attributes {dlti.dl_spec = #dlti.dl_spec<#dlti.dl_entry<index, 32 : i32>>}
+// CHECK-DL-LABEL: gpu.module @launch_memory_attributions_1_kernel attributes {dlti.dl_spec = #dlti.dl_spec<index = 32 : i32>}
 
 // -----
 // CHECK: module attributes {gpu.container_module}
diff --git a/mlir/test/Target/LLVMIR/Import/data-layout.ll b/mlir/test/Target/LLVMIR/Import/data-layout.ll
index ee6f4dd994f1d0..c397053585e3c9 100644
--- a/mlir/test/Target/LLVMIR/Import/data-layout.ll
+++ b/mlir/test/Target/LLVMIR/Import/data-layout.ll
@@ -4,16 +4,16 @@
 
 ; CHECK: dlti.dl_spec =
 ; CHECK: #dlti.dl_spec<
-; CHECK-DAG:   #dlti.dl_entry<"dlti.endianness", "little">
-; CHECK-DAG:   #dlti.dl_entry<i1, dense<8> : vector<2xi64>>
-; CHECK-DAG:   #dlti.dl_entry<i8, dense<8> : vector<2xi64>>
-; CHECK-DAG:   #dlti.dl_entry<i16, dense<16> : vector<2xi64>>
-; CHECK-DAG:   #dlti.dl_entry<i32, dense<32> : vector<2xi64>>
-; CHECK-DAG:   #dlti.dl_entry<i64, dense<[32, 64]> : vector<2xi64>>
-; CHECK-DAG:   #dlti.dl_entry<!llvm.ptr, dense<64> : vector<4xi64>>
-; CHECK-DAG:   #dlti.dl_entry<f16, dense<16> : vector<2xi64>>
-; CHECK-DAG:   #dlti.dl_entry<f64, dense<64> : vector<2xi64>>
-; CHECK-DAG:   #dlti.dl_entry<f128, dense<128> : vector<2xi64>>
+; CHECK-DAG:   "dlti.endianness" = "little"
+; CHECK-DAG:   i1 = dense<8> : vector<2xi64>
+; CHECK-DAG:   i8 = dense<8> : vector<2xi64>
+; CHECK-DAG:   i16 = dense<16> : vector<2xi64>
+; CHECK-DAG:   i32 = dense<32> : vector<2xi64>
+; CHECK-DAG:   i64 = dense<[32, 64]> : vector<2xi64>
+; CHECK-DAG:   !llvm.ptr = dense<64> : vector<4xi64>
+; CHECK-DAG:   f16 = dense<16> : vector<2xi64>
+; CHECK-DAG:   f64 = dense<64> : vector<2xi64>
+; CHECK-DAG:   f128 = dense<128> : vector<2xi64>
 ; CHECK: >
 target datalayout = ""
 
@@ -21,30 +21,30 @@ target datalayout = ""
 
 ; CHECK: dlti.dl_spec =
 ; CHECK: #dlti.dl_spec<
-; CHECK-DAG:   #dlti.dl_entry<"dlti.endianness", "little">
-; CHECK-DAG:   #dlti.dl_entry<i64, dense<64> : vector<2xi64>>
-; CHECK-DAG:   #dlti.dl_entry<f80, dense<128> : vector<2xi64>>
-; CHECK-DAG:   #dlti.dl_entry<i8, dense<8> : vector<2xi64>>
-; CHECK-DAG:   #dlti.dl_entry<!llvm.ptr<270>, dense<[32, 64, 64, 32]> : vector<4xi64>>
-; CHECK-DAG:   #dlti.dl_entry<!llvm.ptr<271>, dense<32> : vector<4xi64>>
-; CHECK-DAG:   #dlti.dl_entry<!llvm.ptr<272>, dense<64> : vector<4xi64>>
-; CHECK-DAG:   #dlti.dl_entry<"dlti.stack_alignment", 128 : i64>
+; CHECK-DAG:   "dlti.endianness" = "little"
+; CHECK-DAG:   i64 = dense<64> : vector<2xi64>
+; CHECK-DAG:   f80 = dense<128> : vector<2xi64>
+; CHECK-DAG:   i8 = dense<8> : vector<2xi64>
+; CHECK-DAG:   !llvm.ptr<270> = dense<[32, 64, 64, 32]> : vector<4xi64>
+; CHECK-DAG:   !llvm.ptr<271> = dense<32> : vector<4xi64>
+; CHECK-DAG:   !llvm.ptr<272> = dense<64> : vector<4xi64>
+; CHECK-DAG:   "dlti.stack_alignment" = 128 : i64
 target datalayout = "e-m:e-p270:32:64-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 
 ; // -----
 
 ; CHECK: dlti.dl_spec =
 ; CHECK: #dlti.dl_spec<
-; CHECK-DAG:   #dlti.dl_entry<"dlti.endianness", "big">
-; CHECK-DAG:   #dlti.dl_entry<!llvm.ptr<270>, dense<[16, 32, 64, 8]> : vector<4xi64>>
-; CHECK-DAG:   #dlti.dl_entry<!llvm.ptr<271>, dense<[16, 32, 64, 16]> : vector<4xi64>>
-; CHECK-DAG:   #dlti.dl_entry<"dlti.alloca_memory_space", 1 : ui64>
-; CHECK-DAG:   #dlti.dl_entry<i64, dense<[64, 128]> : vector<2xi64>>
+; CHECK-DAG:   "dlti.endianness" = "big"
+; CHECK-DAG:   !llvm.ptr<270> = dense<[16, 32, 64, 8]> : vector<4xi64>
+; CHECK-DAG:   !llvm.ptr<271> = dense<[16, 32, 64, 16]> : vector<4xi64>
+; CHECK-DAG:   "dlti.alloca_memory_space" = 1 : ui64
+; CHECK-DAG:   i64 = dense<[64, 128]> : vector<2xi64>
 target datalayout = "A1-E-p270:16:32:64:8-p271:16:32:64-i64:64:128"
 
 ; // -----
 
 ; CHECK: dlti.dl_spec =
 ; CHECK: #dlti.dl_spec<
-; CHECK-NOT:   #dlti.dl_entry<"dlti.alloca_memory_space"
+; CHECK-NOT:   "dlti.alloca_memory_space" =
 target datalayout = "A0"
diff --git a/mlir/unittests/Interfaces/DataLayoutInterfacesTest.cpp b/mlir/unittests/Interfaces/DataLayoutInterfacesTest.cpp
index b667785c16f162..db294b8b040e9d 100644
--- a/mlir/unittests/Interfaces/DataLayoutInterfacesTest.cpp
+++ b/mlir/unittests/Interfaces/DataLayoutInterfacesTest.cpp
@@ -99,9 +99,9 @@ struct CustomDataLayoutSpec
 
 class TargetSystemSpecStorage : public AttributeStorage {
 public:
-  using KeyTy = ArrayRef<DeviceIDTargetDeviceSpecPair>;
+  using KeyTy = ArrayRef<DataLayoutEntryInterface>;
 
-  TargetSystemSpecStorage(ArrayRef<DeviceIDTargetDeviceSpecPair> entries)
+  TargetSystemSpecStorage(ArrayRef<DataLayoutEntryInterface> entries)
       : entries(entries) {}
 
   bool operator==(const KeyTy &key) const { return key == entries; }
@@ -112,7 +112,7 @@ class TargetSystemSpecStorage : public AttributeStorage {
         TargetSystemSpecStorage(allocator.copyInto(key));
   }
 
-  ArrayRef<DeviceIDTargetDeviceSpecPair> entries;
+  ArrayRef<DataLayoutEntryInterface> entries;
 };
 
 struct CustomTargetSystemSpec
@@ -126,18 +126,20 @@ struct CustomTargetSystemSpec
   static constexpr StringLiteral name = "test.custom_target_system_spec";
 
   static CustomTargetSystemSpec
-  get(MLIRContext *ctx, ArrayRef<DeviceIDTargetDeviceSpecPair> entries) {
+  get(MLIRContext *ctx, ArrayRef<DataLayoutEntryInterface> entries) {
     return Base::get(ctx, entries);
   }
-  DeviceIDTargetDeviceSpecPairListRef getEntries() const {
+  ArrayRef<DataLayoutEntryInterface> getEntries() const {
     return getImpl()->entries;
   }
   LogicalResult verifySpec(Location loc) { return success(); }
   std::optional<TargetDeviceSpecInterface>
   getDeviceSpecForDeviceID(TargetSystemSpecInterface::DeviceID deviceID) {
     for (const auto &entry : getEntries()) {
-      if (entry.first == deviceID)
-        return entry.second;
+      if (entry.getKey() == DataLayoutEntryKey(deviceID))
+        if (auto deviceSpec =
+                llvm::dyn_cast<TargetDeviceSpecInterface>(entry.getValue()))
+          return deviceSpec;
     }
     return std::nullopt;
   }
@@ -388,9 +390,11 @@ struct DLTargetSystemDescTestDialect : public Dialect {
   void printAttribute(Attribute attr,
                       DialectAsmPrinter &printer) const override {
     printer << "target_system_spec<";
-    llvm::interleaveComma(
-        cast<CustomTargetSystemSpec>(attr).getEntries(), printer,
-        [&](const auto &it) { printer << it.first << ":" << it.second; });
+    llvm::interleaveComma(cast<CustomTargetSystemSpec>(attr).getEntries(),
+                          printer, [&](const auto &it) {
+                            printer << dyn_cast<StringAttr>(it.getKey()) << ":"
+                                    << it.getValue();
+                          });
     printer << ">";
   }
 
@@ -402,8 +406,8 @@ struct DLTargetSystemDescTestDialect : public Dialect {
     if (succeeded(parser.parseOptionalGreater()))
       return CustomTargetSystemSpec::get(parser.getContext(), {});
 
-    auto parseDeviceIDTargetDeviceSpecPair =
-        [&](AsmParser &parser) -> FailureOr<DeviceIDTargetDeviceSpecPair> {
+    auto parseTargetDeviceSpecEntry =
+        [&](AsmParser &parser) -> FailureOr<TargetDeviceSpecEntry> {
       std::string deviceID;
       if (failed(parser.parseString(&deviceID))) {
         parser.emitError(parser.getCurrentLocation())
@@ -425,13 +429,15 @@ struct DLTargetSystemDescTestDialect : public Dialect {
                             targetDeviceSpec);
     };
 
-    SmallVector<DeviceIDTargetDeviceSpecPair> entries;
+    SmallVector<DataLayoutEntryInterface> entries;
     ok = succeeded(parser.parseCommaSeparatedList([&]() {
-      auto deviceIDAndTargetDeviceSpecPair =
-          parseDeviceIDTargetDeviceSpecPair(parser);
+      auto deviceIDAndTargetDeviceSpecPair = parseTargetDeviceSpecEntry(parser);
       ok = succeeded(deviceIDAndTargetDeviceSpecPair);
       assert(ok);
-      entries.push_back(*deviceIDAndTargetDeviceSpecPair);
+      auto entry =
+          DataLayoutEntryAttr::get(deviceIDAndTargetDeviceSpecPair->first,
+                                   deviceIDAndTargetDeviceSpecPair->second);
+      entries.push_back(entry);
       return success();
     }));
     assert(ok);

>From c33c4bfea2c187af36bf8e35698b497f0e5addb0 Mon Sep 17 00:00:00 2001
From: Rolf Morel <rolf.morel at intel.com>
Date: Wed, 30 Oct 2024 11:37:16 -0700
Subject: [PATCH 2/3] Address @Dinistro and @ftynse's comments and fix flang
 DLTI tests

---
 flang/test/Fir/tco-default-datalayout.fir     |  2 +-
 flang/test/Fir/tco-explicit-datalayout.fir    |  2 +-
 mlir/include/mlir/Dialect/DLTI/DLTIAttrs.td   |  8 ++---
 .../mlir/Interfaces/DataLayoutInterfaces.td   |  6 ++--
 mlir/lib/Dialect/DLTI/DLTI.cpp                | 36 ++++++++++++-------
 mlir/lib/Interfaces/DataLayoutInterfaces.cpp  |  2 +-
 6 files changed, 34 insertions(+), 22 deletions(-)

diff --git a/flang/test/Fir/tco-default-datalayout.fir b/flang/test/Fir/tco-default-datalayout.fir
index 0741e820a8d19d..c6a4ddb46853f2 100644
--- a/flang/test/Fir/tco-default-datalayout.fir
+++ b/flang/test/Fir/tco-default-datalayout.fir
@@ -7,6 +7,6 @@ module {
 // CHECK: module attributes {
 // CHECK-SAME: dlti.dl_spec = #dlti.dl_spec<
 // ...
-// CHECK-SAME:    #dlti.dl_entry<i64, dense<[32, 64]> : vector<2xi64>>,
+// CHECK-SAME:    i64 = dense<[32, 64]> : vector<2xi64>,
 // ...
 // CHECK-SAME:    llvm.data_layout = ""
diff --git a/flang/test/Fir/tco-explicit-datalayout.fir b/flang/test/Fir/tco-explicit-datalayout.fir
index 50d8d835a602f3..cae500a948aa5c 100644
--- a/flang/test/Fir/tco-explicit-datalayout.fir
+++ b/flang/test/Fir/tco-explicit-datalayout.fir
@@ -8,6 +8,6 @@ module attributes {llvm.data_layout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i6
 // CHECK: module attributes {
 // CHECK-SAME: dlti.dl_spec = #dlti.dl_spec<
 // ...
-// CHECK-SAME:    #dlti.dl_entry<i64, dense<128> : vector<2xi64>>,
+// CHECK-SAME:    i64 = dense<128> : vector<2xi64>,
 // ...
 // CHECK-SAME:    llvm.data_layout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:128-i128:128-f80:128-n8:16:32:64-S128"
diff --git a/mlir/include/mlir/Dialect/DLTI/DLTIAttrs.td b/mlir/include/mlir/Dialect/DLTI/DLTIAttrs.td
index 1caf5fd8787c7b..f8950c974eb693 100644
--- a/mlir/include/mlir/Dialect/DLTI/DLTIAttrs.td
+++ b/mlir/include/mlir/Dialect/DLTI/DLTIAttrs.td
@@ -88,7 +88,7 @@ def DLTI_DataLayoutSpecAttr :
 
     /// Returns the attribute associated with the key.
     FailureOr<Attribute> query(DataLayoutEntryKey key) {
-      return llvm::cast<mlir::DataLayoutSpecInterface>(*this).queryHelper(key);
+      return ::llvm::cast<mlir::DataLayoutSpecInterface>(*this).queryHelper(key);
     }
   }];
 }
@@ -194,7 +194,7 @@ def DLTI_TargetSystemSpecAttr :
 
     /// Returns the attribute associated with the key.
     FailureOr<Attribute> query(DataLayoutEntryKey key) const {
-      return llvm::cast<mlir::TargetSystemSpecInterface>(*this).queryHelper(key);
+      return ::llvm::cast<mlir::TargetSystemSpecInterface>(*this).queryHelper(key);
     }
   }];
   let extraClassDefinition = [{
@@ -204,7 +204,7 @@ def DLTI_TargetSystemSpecAttr :
       for (const auto& entry : getEntries()) {
         if (entry.getKey() == DataLayoutEntryKey(deviceID))
           if (auto deviceSpec =
-              llvm::dyn_cast<TargetDeviceSpecInterface>(entry.getValue()))
+              ::llvm::dyn_cast<TargetDeviceSpecInterface>(entry.getValue()))
             return deviceSpec;
       }
       return std::nullopt;
@@ -239,7 +239,7 @@ def DLTI_TargetDeviceSpecAttr :
   let extraClassDeclaration = [{
     /// Returns the attribute associated with the key.
     FailureOr<Attribute> query(DataLayoutEntryKey key) const {
-      return llvm::cast<mlir::TargetDeviceSpecInterface>(*this).queryHelper(key);
+      return ::llvm::cast<mlir::TargetDeviceSpecInterface>(*this).queryHelper(key);
     }
   }];
 }
diff --git a/mlir/include/mlir/Interfaces/DataLayoutInterfaces.td b/mlir/include/mlir/Interfaces/DataLayoutInterfaces.td
index 061dee2399d9ad..3532116700af51 100644
--- a/mlir/include/mlir/Interfaces/DataLayoutInterfaces.td
+++ b/mlir/include/mlir/Interfaces/DataLayoutInterfaces.td
@@ -276,7 +276,7 @@ def TargetDeviceSpecInterface : AttrInterface<"TargetDeviceSpecInterface", [DLTI
     /// Helper for default implementation of `DLTIQueryInterface`'s `query`.
     ::mlir::FailureOr<::mlir::Attribute>
     queryHelper(::mlir::DataLayoutEntryKey key) const {
-      if (auto strKey = llvm::dyn_cast<StringAttr>(key))
+      if (auto strKey = ::llvm::dyn_cast<StringAttr>(key))
         if (DataLayoutEntryInterface spec = getSpecForIdentifier(strKey))
           return spec.getValue();
       return ::mlir::failure();
@@ -304,7 +304,7 @@ def TargetSystemSpecInterface : AttrInterface<"TargetSystemSpecInterface", [DLTI
   let methods = [
     InterfaceMethod<
       /*description=*/"Returns the list of layout entries.",
-      /*retTy=*/"llvm::ArrayRef<DataLayoutEntryInterface>",
+      /*retTy=*/"::llvm::ArrayRef<DataLayoutEntryInterface>",
       /*methodName=*/"getEntries",
       /*args=*/(ins)
     >,
@@ -334,7 +334,7 @@ def TargetSystemSpecInterface : AttrInterface<"TargetSystemSpecInterface", [DLTI
     /// Helper for default implementation of `DLTIQueryInterface`'s `query`.
     ::mlir::FailureOr<::mlir::Attribute>
     queryHelper(::mlir::DataLayoutEntryKey key) const {
-      if (auto strKey = llvm::dyn_cast<::mlir::StringAttr>(key))
+      if (auto strKey = ::llvm::dyn_cast<::mlir::StringAttr>(key))
         if (auto deviceSpec = getDeviceSpecForDeviceID(strKey))
           return *deviceSpec;
       return ::mlir::failure();
diff --git a/mlir/lib/Dialect/DLTI/DLTI.cpp b/mlir/lib/Dialect/DLTI/DLTI.cpp
index d8946d865a1836..e6bd2309ddbe14 100644
--- a/mlir/lib/Dialect/DLTI/DLTI.cpp
+++ b/mlir/lib/Dialect/DLTI/DLTI.cpp
@@ -30,9 +30,13 @@ using namespace mlir;
 #define DEBUG_TYPE "dlti"
 
 //===----------------------------------------------------------------------===//
-// parsing
+// Common parsing utility functions.
 //===----------------------------------------------------------------------===//
 
+/// Parse an entry which can either be of the form `key = value` or a
+/// #dlti.dl_entry attribute. When `tryType=true` the key can be a type,
+/// otherwise only quoted strings are allowed. The grammar is as follows:
+///   entry ::= ((type | quoted-string) `=` attr) | dl-entry-attr
 static ParseResult parseKeyValuePair(AsmParser &parser,
                                      DataLayoutEntryInterface &entry,
                                      bool tryType = false) {
@@ -79,6 +83,12 @@ static ParseResult parseKeyValuePair(AsmParser &parser,
          << "failed to parse DLTI entry";
 }
 
+/// Construct a requested attribute by parsing list of entries occurring within
+/// a pair of `<` and `>`, optionally allow types as keys and an empty list.
+/// The grammar is as follows:
+///   bracketed-entry-list ::=`<` entry-list `>`
+///   entry-list ::= | entry | entry `,` entry-list
+///   entry ::= ((type | quoted-string) `=` attr) | dl-entry-attr
 template <class Attr>
 static Attribute parseAngleBracketedEntries(AsmParser &parser, Type ty,
                                             bool tryType = false,
@@ -100,18 +110,19 @@ static Attribute parseAngleBracketedEntries(AsmParser &parser, Type ty,
 }
 
 //===----------------------------------------------------------------------===//
-// printing
+// Common printing utility functions.
 //===----------------------------------------------------------------------===//
 
+/// Convert pointer-union keys to strings.
 static inline std::string keyToStr(DataLayoutEntryKey key) {
   std::string buf;
-  llvm::TypeSwitch<DataLayoutEntryKey>(key)
+  TypeSwitch<DataLayoutEntryKey>(key)
       .Case<StringAttr, Type>( // The only two kinds of key we know of.
-          [&](auto key) { llvm::raw_string_ostream(buf) << key; })
-      .Default([](auto) { llvm_unreachable("unexpected entry key kind"); });
+          [&](auto key) { llvm::raw_string_ostream(buf) << key; });
   return buf;
 }
 
+/// Pretty-print entries, each in `key = value` format, separated by commas.
 template <class T>
 static void printAngleBracketedEntries(AsmPrinter &os, T &&entries) {
   os << "<";
@@ -122,9 +133,10 @@ static void printAngleBracketedEntries(AsmPrinter &os, T &&entries) {
 }
 
 //===----------------------------------------------------------------------===//
-// verifying
+// Common verifying utility functions.
 //===----------------------------------------------------------------------===//
 
+/// Verify entries, with the option to disallow types as keys.
 static LogicalResult verifyEntries(function_ref<InFlightDiagnostic()> emitError,
                                    ArrayRef<DataLayoutEntryInterface> entries,
                                    bool allowTypes = true) {
@@ -135,7 +147,7 @@ static LogicalResult verifyEntries(function_ref<InFlightDiagnostic()> emitError,
     DataLayoutEntryKey key = entry.getKey();
     if (key.isNull())
       return emitError() << "contained invalid DLTI key";
-    if (!allowTypes && llvm::dyn_cast<Type>(key))
+    if (!allowTypes && dyn_cast<Type>(key))
       return emitError() << "type as DLIT key is not allowed";
     if (!keys.insert(key).second)
       return emitError() << "repeated DLTI key: " << keyToStr(key);
@@ -306,7 +318,7 @@ combineOneSpec(DataLayoutSpecInterface spec,
                typeSample.getContext()->getLoadedDialect<BuiltinDialect>() &&
            "unexpected data layout entry for built-in type");
 
-    auto interface = llvm::cast<DataLayoutTypeInterface>(typeSample);
+    auto interface = cast<DataLayoutTypeInterface>(typeSample);
     if (!interface.areCompatible(entriesForType.lookup(kvp.first), kvp.second))
       return failure();
 
@@ -340,7 +352,7 @@ DataLayoutSpecAttr
 DataLayoutSpecAttr::combineWith(ArrayRef<DataLayoutSpecInterface> specs) const {
   // Only combine with attributes of the same kind.
   // TODO: reconsider this when the need arises.
-  if (llvm::any_of(specs, [](DataLayoutSpecInterface spec) {
+  if (any_of(specs, [](DataLayoutSpecInterface spec) {
         return !llvm::isa<DataLayoutSpecAttr>(spec);
       }))
     return {};
@@ -488,7 +500,7 @@ getClosestQueryable(Operation *op) {
   // Search op and its ancestors for the first attached DLTIQueryInterface attr.
   do {
     for (NamedAttribute attr : op->getAttrs())
-      if ((queryable = llvm::dyn_cast<DLTIQueryInterface>(attr.getValue())))
+      if ((queryable = dyn_cast<DLTIQueryInterface>(attr.getValue())))
         break;
   } while (!queryable && (op = op->getParentOp()));
 
@@ -519,7 +531,7 @@ dlti::query(Operation *op, ArrayRef<DataLayoutEntryKey> keys, bool emitError) {
 
   Attribute currentAttr = queryable;
   for (auto &&[idx, key] : llvm::enumerate(keys)) {
-    if (auto map = llvm::dyn_cast<DLTIQueryInterface>(currentAttr)) {
+    if (auto map = dyn_cast<DLTIQueryInterface>(currentAttr)) {
       auto maybeAttr = map.query(key);
       if (failed(maybeAttr)) {
         if (emitError) {
@@ -565,7 +577,7 @@ class TargetDataLayoutInterface : public DataLayoutDialectInterface {
                             Location loc) const final {
     StringRef entryName = entry.getKey().get<StringAttr>().strref();
     if (entryName == DLTIDialect::kDataLayoutEndiannessKey) {
-      auto value = llvm::dyn_cast<StringAttr>(entry.getValue());
+      auto value = dyn_cast<StringAttr>(entry.getValue());
       if (value &&
           (value.getValue() == DLTIDialect::kDataLayoutEndiannessBig ||
            value.getValue() == DLTIDialect::kDataLayoutEndiannessLittle))
diff --git a/mlir/lib/Interfaces/DataLayoutInterfaces.cpp b/mlir/lib/Interfaces/DataLayoutInterfaces.cpp
index 1c22945c4caaf7..9469780129d644 100644
--- a/mlir/lib/Interfaces/DataLayoutInterfaces.cpp
+++ b/mlir/lib/Interfaces/DataLayoutInterfaces.cpp
@@ -791,7 +791,7 @@ mlir::detail::verifyTargetSystemSpec(TargetSystemSpecInterface spec,
   DenseSet<TargetSystemSpecInterface::DeviceID> deviceIDs;
   for (const auto &entry : spec.getEntries()) {
     auto targetDeviceSpec =
-        llvm::dyn_cast<TargetDeviceSpecInterface>(entry.getValue());
+        dyn_cast<TargetDeviceSpecInterface>(entry.getValue());
 
     if (!targetDeviceSpec)
       return failure();

>From 7b41f2db683dbf5e900511a30f7e3d494cb7be02 Mon Sep 17 00:00:00 2001
From: Rolf Morel <rolf.morel at intel.com>
Date: Wed, 30 Oct 2024 12:22:19 -0700
Subject: [PATCH 3/3] Place checking for empty string in verifier

---
 mlir/lib/Dialect/DLTI/DLTI.cpp      | 11 +++++------
 mlir/test/Dialect/DLTI/invalid.mlir |  5 +++++
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/mlir/lib/Dialect/DLTI/DLTI.cpp b/mlir/lib/Dialect/DLTI/DLTI.cpp
index e6bd2309ddbe14..ec14689f4a273d 100644
--- a/mlir/lib/Dialect/DLTI/DLTI.cpp
+++ b/mlir/lib/Dialect/DLTI/DLTI.cpp
@@ -60,11 +60,7 @@ static ParseResult parseKeyValuePair(AsmParser &parser,
 
   std::string ident;
   OptionalParseResult parsedStr = parser.parseOptionalString(&ident);
-  if (parsedStr.has_value() && !ident.empty()) {
-    if (failed(parsedStr.value()))
-      return parser.emitError(parser.getCurrentLocation())
-             << "error while parsing string DLTI key";
-
+  if (parsedStr.has_value() && succeeded(parsedStr.value())) {
     if (failed(parser.parseEqual()) || failed(parser.parseAttribute(value)))
       return failure(); // Assume that an error has already been emitted.
 
@@ -148,7 +144,10 @@ static LogicalResult verifyEntries(function_ref<InFlightDiagnostic()> emitError,
     if (key.isNull())
       return emitError() << "contained invalid DLTI key";
     if (!allowTypes && dyn_cast<Type>(key))
-      return emitError() << "type as DLIT key is not allowed";
+      return emitError() << "type as DLTI key is not allowed";
+    if (auto strKey = dyn_cast<StringAttr>(key))
+      if (strKey.getValue().empty())
+        return emitError() << "empty string as DLTI key is not allowed";
     if (!keys.insert(key).second)
       return emitError() << "repeated DLTI key: " << keyToStr(key);
     if (!entry.getValue())
diff --git a/mlir/test/Dialect/DLTI/invalid.mlir b/mlir/test/Dialect/DLTI/invalid.mlir
index ff0b435613fb17..2436e4f7484f5a 100644
--- a/mlir/test/Dialect/DLTI/invalid.mlir
+++ b/mlir/test/Dialect/DLTI/invalid.mlir
@@ -25,6 +25,11 @@
 
 // -----
 
+// expected-error at below {{empty string as DLTI key is not allowed}}
+"test.unknown_op"() { test.unknown_attr = #dlti.map<"" = 42> } : () -> ()
+
+// -----
+
 // expected-error at below {{repeated DLTI key: "test.id"}}
 "test.unknown_op"() { test.unknown_attr = #dlti.dl_spec<
   #dlti.dl_entry<"test.id", 42>,



More information about the Mlir-commits mailing list