diff --git a/mlir/include/mlir/Interfaces/DataLayoutInterfaces.h b/mlir/include/mlir/Interfaces/DataLayoutInterfaces.h index 4cbad38df42bab..ab65f92820a6a8 100644 --- a/mlir/include/mlir/Interfaces/DataLayoutInterfaces.h +++ b/mlir/include/mlir/Interfaces/DataLayoutInterfaces.h @@ -93,8 +93,7 @@ uint64_t getDefaultStackAlignment(DataLayoutEntryInterface entry); /// Returns the value of the property from the specified DataLayoutEntry. If the /// property is missing from the entry, returns std::nullopt. -std::optional -getDevicePropertyValueAsInt(DataLayoutEntryInterface entry); +std::optional getDevicePropertyValue(DataLayoutEntryInterface entry); /// Given a list of data layout entries, returns a new list containing the /// entries with keys having the given type ID, i.e. belonging to the same type @@ -246,9 +245,9 @@ class DataLayout { /// Returns the value of the specified property if the property is defined for /// the given device ID, otherwise returns std::nullopt. - std::optional - getDevicePropertyValueAsInt(TargetSystemSpecInterface::DeviceID, - StringAttr propertyName) const; + std::optional + getDevicePropertyValue(TargetSystemSpecInterface::DeviceID, + StringAttr propertyName) const; private: /// Combined layout spec at the given scope. diff --git a/mlir/include/mlir/Interfaces/DataLayoutInterfaces.td b/mlir/include/mlir/Interfaces/DataLayoutInterfaces.td index 8ff0e3f5f863b6..d4b89997d9c867 100644 --- a/mlir/include/mlir/Interfaces/DataLayoutInterfaces.td +++ b/mlir/include/mlir/Interfaces/DataLayoutInterfaces.td @@ -468,12 +468,12 @@ def DataLayoutOpInterface : OpInterface<"DataLayoutOpInterface"> { StaticInterfaceMethod< /*description=*/"Returns the value of the property, if the property is " "defined. Otherwise, it returns std::nullopt.", - /*retTy=*/"std::optional", - /*methodName=*/"getDevicePropertyValueAsInt", + /*retTy=*/"std::optional", + /*methodName=*/"getDevicePropertyValue", /*args=*/(ins "::mlir::DataLayoutEntryInterface":$entry), /*methodBody=*/"", /*defaultImplementation=*/[{ - return ::mlir::detail::getDevicePropertyValueAsInt(entry); + return ::mlir::detail::getDevicePropertyValue(entry); }] > ]; diff --git a/mlir/lib/Dialect/DLTI/DLTI.cpp b/mlir/lib/Dialect/DLTI/DLTI.cpp index 592fced0b2abdd..420c605d1a19b2 100644 --- a/mlir/lib/Dialect/DLTI/DLTI.cpp +++ b/mlir/lib/Dialect/DLTI/DLTI.cpp @@ -352,6 +352,7 @@ TargetDeviceSpecAttr::verify(function_ref 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(); if (!ids.insert(id).second) return emitError() << "repeated layout entry key: " << id.getValue(); diff --git a/mlir/lib/Interfaces/DataLayoutInterfaces.cpp b/mlir/lib/Interfaces/DataLayoutInterfaces.cpp index df86bd757b6282..2634245a4b7b1e 100644 --- a/mlir/lib/Interfaces/DataLayoutInterfaces.cpp +++ b/mlir/lib/Interfaces/DataLayoutInterfaces.cpp @@ -293,13 +293,12 @@ mlir::detail::getDefaultStackAlignment(DataLayoutEntryInterface entry) { return value.getValue().getZExtValue(); } -std::optional -mlir::detail::getDevicePropertyValueAsInt(DataLayoutEntryInterface entry) { +std::optional +mlir::detail::getDevicePropertyValue(DataLayoutEntryInterface entry) { if (entry == DataLayoutEntryInterface()) return std::nullopt; - auto value = cast(entry.getValue()); - return value.getValue().getZExtValue(); + return entry.getValue(); } DataLayoutEntryList @@ -661,7 +660,7 @@ uint64_t mlir::DataLayout::getStackAlignment() const { return *stackAlignment; } -std::optional mlir::DataLayout::getDevicePropertyValueAsInt( +std::optional mlir::DataLayout::getDevicePropertyValue( TargetSystemSpecInterface::DeviceID deviceID, StringAttr propertyName) const { checkValid(); @@ -676,9 +675,9 @@ std::optional mlir::DataLayout::getDevicePropertyValueAsInt( // missing, we return std::nullopt so that the users can resort to // the default value however they want. if (auto iface = dyn_cast_or_null(scope)) - return iface.getDevicePropertyValueAsInt(entry); + return iface.getDevicePropertyValue(entry); else - return detail::getDevicePropertyValueAsInt(entry); + return detail::getDevicePropertyValue(entry); } //===----------------------------------------------------------------------===// diff --git a/mlir/test/Dialect/DLTI/invalid.mlir b/mlir/test/Dialect/DLTI/invalid.mlir index d732fb462e68e9..36bf6088fab7e9 100644 --- a/mlir/test/Dialect/DLTI/invalid.mlir +++ b/mlir/test/Dialect/DLTI/invalid.mlir @@ -126,7 +126,7 @@ module attributes { // expected-error@+2 {{failed to parse DLTI_TargetSystemSpecAttr parameter 'entries' which is to be a `::llvm::ArrayRef`}} dlti.target_system_spec = #dlti.target_system_spec< 0: #dlti.target_device_spec< - #dlti.dl_entry<"L1_cache_size_in_bytes", 4096: i32>> + #dlti.dl_entry<"L1_cache_size_in_bytes", 4096 : i32>> >} {} // ----- diff --git a/mlir/test/Dialect/DLTI/roundtrip.mlir b/mlir/test/Dialect/DLTI/roundtrip.mlir index 7b8255ad71d4d9..43188aad595a7a 100644 --- a/mlir/test/Dialect/DLTI/roundtrip.mlir +++ b/mlir/test/Dialect/DLTI/roundtrip.mlir @@ -66,8 +66,8 @@ 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>>, + #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>> + #dlti.dl_entry<"dlti.max_vector_op_width", 128 : ui32>> >} {} diff --git a/mlir/test/Dialect/DLTI/valid.mlir b/mlir/test/Dialect/DLTI/valid.mlir index 98e9d0b8de4913..8ee7a7eaa069db 100644 --- a/mlir/test/Dialect/DLTI/valid.mlir +++ b/mlir/test/Dialect/DLTI/valid.mlir @@ -13,9 +13,9 @@ module attributes { dlti.target_system_spec = #dlti.target_system_spec< "CPU": #dlti.target_device_spec< - #dlti.dl_entry<"L1_cache_size_in_bytes", 4096: i32>>, + #dlti.dl_entry<"L1_cache_size_in_bytes", 4096 : i32>>, "GPU": #dlti.target_device_spec< - #dlti.dl_entry<"max_vector_op_width", 128: i32>> + #dlti.dl_entry<"max_vector_op_width", 128 : i32>> >} {} // ----- @@ -31,9 +31,9 @@ module attributes { module attributes { dlti.target_system_spec = #dlti.target_system_spec< "CPU": #dlti.target_device_spec< - #dlti.dl_entry<"L1_cache_size_in_bytes", 4096: i32>>, + #dlti.dl_entry<"L1_cache_size_in_bytes", 4096 : i32>>, "GPU": #dlti.target_device_spec< - #dlti.dl_entry<"L1_cache_size_in_bytes", 8192: i32>> + #dlti.dl_entry<"L1_cache_size_in_bytes", 8192 : i32>> >} {} // ----- @@ -49,9 +49,9 @@ module attributes { module attributes { dlti.target_system_spec = #dlti.target_system_spec< "CPU": #dlti.target_device_spec< - #dlti.dl_entry<"L1_cache_size_in_bytes", 4096: i64>>, + #dlti.dl_entry<"L1_cache_size_in_bytes", 4096 : i64>>, "GPU": #dlti.target_device_spec< - #dlti.dl_entry<"L1_cache_size_in_bytes", 8192: i64>> + #dlti.dl_entry<"L1_cache_size_in_bytes", 8192 : i64>> >} {} // ----- @@ -67,9 +67,9 @@ module attributes { module attributes { dlti.target_system_spec = #dlti.target_system_spec< "CPU": #dlti.target_device_spec< - #dlti.dl_entry<"max_vector_op_width", 64: i32>>, + #dlti.dl_entry<"max_vector_op_width", 64 : i32>>, "GPU": #dlti.target_device_spec< - #dlti.dl_entry<"max_vector_op_width", 128: i32>> + #dlti.dl_entry<"max_vector_op_width", 128 : i32>> >} {} // ----- @@ -85,9 +85,9 @@ module attributes { module attributes { dlti.target_system_spec = #dlti.target_system_spec< "CPU": #dlti.target_device_spec< - #dlti.dl_entry<"max_vector_op_width", 64: i64>>, + #dlti.dl_entry<"max_vector_op_width", 64 : i64>>, "GPU": #dlti.target_device_spec< - #dlti.dl_entry<"max_vector_op_width", 128: i64>> + #dlti.dl_entry<"max_vector_op_width", 128 : i64>> >} {} // ----- @@ -103,7 +103,47 @@ module attributes { module attributes { dlti.target_system_spec = #dlti.target_system_spec< "CPU": #dlti.target_device_spec< - #dlti.dl_entry<"max_vector_op_width", 64>>, + #dlti.dl_entry<"max_vector_op_width", 64 : i64>>, "GPU": #dlti.target_device_spec< - #dlti.dl_entry<"max_vector_op_width", 128>> + #dlti.dl_entry<"max_vector_op_width", 128 : i64>> + >} {} + +// ----- + +// 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: } +module attributes { + dlti.target_system_spec = #dlti.target_system_spec< + "CPU": #dlti.target_device_spec< + #dlti.dl_entry<"L1_cache_size_in_bytes", 4096 : ui32>>, + "GPU": #dlti.target_device_spec< + #dlti.dl_entry<"max_vector_op_width", "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: #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: } +module attributes { + dlti.target_system_spec = #dlti.target_system_spec< + "CPU": #dlti.target_device_spec< + #dlti.dl_entry<"max_vector_op_width", 4096.0 : f32>>, + "GPU": #dlti.target_device_spec< + #dlti.dl_entry<"L1_cache_size_in_bytes", "128">> >} {} diff --git a/mlir/unittests/Interfaces/DataLayoutInterfacesTest.cpp b/mlir/unittests/Interfaces/DataLayoutInterfacesTest.cpp index bcbfa88c5d8159..d1227b045d4ed3 100644 --- a/mlir/unittests/Interfaces/DataLayoutInterfacesTest.cpp +++ b/mlir/unittests/Interfaces/DataLayoutInterfacesTest.cpp @@ -495,11 +495,11 @@ TEST(DataLayout, NullSpec) { EXPECT_EQ(layout.getGlobalMemorySpace(), Attribute()); EXPECT_EQ(layout.getStackAlignment(), 0u); - EXPECT_EQ(layout.getDevicePropertyValueAsInt( + EXPECT_EQ(layout.getDevicePropertyValue( Builder(&ctx).getStringAttr("CPU" /* device ID*/), Builder(&ctx).getStringAttr("L1_cache_size_in_bytes")), std::nullopt); - EXPECT_EQ(layout.getDevicePropertyValueAsInt( + EXPECT_EQ(layout.getDevicePropertyValue( Builder(&ctx).getStringAttr("CPU" /* device ID*/), Builder(&ctx).getStringAttr("max_vector_width")), std::nullopt); @@ -535,11 +535,11 @@ TEST(DataLayout, EmptySpec) { EXPECT_EQ(layout.getGlobalMemorySpace(), Attribute()); EXPECT_EQ(layout.getStackAlignment(), 0u); - EXPECT_EQ(layout.getDevicePropertyValueAsInt( + EXPECT_EQ(layout.getDevicePropertyValue( Builder(&ctx).getStringAttr("CPU" /* device ID*/), Builder(&ctx).getStringAttr("L1_cache_size_in_bytes")), std::nullopt); - EXPECT_EQ(layout.getDevicePropertyValueAsInt( + EXPECT_EQ(layout.getDevicePropertyValue( Builder(&ctx).getStringAttr("CPU" /* device ID*/), Builder(&ctx).getStringAttr("max_vector_width")), std::nullopt); @@ -599,8 +599,8 @@ TEST(DataLayout, SpecWithTargetSystemDescEntries) { module attributes { dl_target_sys_desc_test.target_system_spec = #dl_target_sys_desc_test.target_system_spec< "CPU": #dlti.target_device_spec< - #dlti.dl_entry<"L1_cache_size_in_bytes", 4096 : ui32>, - #dlti.dl_entry<"max_vector_op_width", 128 : ui32>> + #dlti.dl_entry<"L1_cache_size_in_bytes", "4096">, + #dlti.dl_entry<"max_vector_op_width", "128">> > } {} )MLIR"; @@ -610,14 +610,14 @@ TEST(DataLayout, SpecWithTargetSystemDescEntries) { OwningOpRef module = parseSourceString(ir, &ctx); DataLayout layout(*module); - EXPECT_EQ(layout.getDevicePropertyValueAsInt( + EXPECT_EQ(layout.getDevicePropertyValue( Builder(&ctx).getStringAttr("CPU") /* device ID*/, Builder(&ctx).getStringAttr("L1_cache_size_in_bytes")), - std::optional(4096)); - EXPECT_EQ(layout.getDevicePropertyValueAsInt( + std::optional(Builder(&ctx).getStringAttr("4096"))); + EXPECT_EQ(layout.getDevicePropertyValue( Builder(&ctx).getStringAttr("CPU") /* device ID*/, Builder(&ctx).getStringAttr("max_vector_op_width")), - std::optional(128)); + std::optional(Builder(&ctx).getStringAttr("128"))); } TEST(DataLayout, Caching) {