Skip to content

Commit fd8f300

Browse files
fabianmcgfabianmcg
committed
address reviewer comments
1 parent 1fbbd81 commit fd8f300

File tree

5 files changed

+47
-56
lines changed

5 files changed

+47
-56
lines changed

‎mlir/include/mlir/Dialect/Ptr/IR/MemorySpaceInterfaces.td

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,6 @@ def MemorySpaceAttrInterface : AttrInterface<"MemorySpaceAttrInterface"> {
3030
}];
3131
let cppNamespace = "::mlir::ptr";
3232
let methods = [
33-
InterfaceMethod<
34-
/*desc=*/ [{
35-
Returns the default memory space as an attribute.
36-
}],
37-
/*returnType=*/ "::mlir::ptr::MemorySpaceAttrInterface",
38-
/*methodName=*/ "getDefaultMemorySpace"
39-
>,
4033
InterfaceMethod<
4134
/*desc=*/ [{
4235
This method checks if it's valid to load a value from the memory space
@@ -96,8 +89,6 @@ def MemorySpaceAttrInterface : AttrInterface<"MemorySpaceAttrInterface"> {
9689
/*desc=*/ [{
9790
This method checks if it's valid to perform an `addrspacecast` op
9891
in the memory space.
99-
Both types are expected to be vectors of rank 1, or scalars of `ptr`
100-
type.
10192
If `emitError` is non-null then the method is allowed to emit errors.
10293
}],
10394
/*returnType=*/ "::mlir::LogicalResult",

‎mlir/include/mlir/Dialect/Ptr/IR/PtrDialect.td

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,11 +57,11 @@ def Ptr_PtrType : Ptr_Type<"Ptr", "ptr", [
5757
let parameters = (ins "MemorySpaceAttrInterface":$memorySpace);
5858
let assemblyFormat = "`<` $memorySpace `>`";
5959
let builders = [
60-
TypeBuilder<(ins CArg<"MemorySpaceAttrInterface", "nullptr">:$memorySpace), [{
61-
return $_get($_ctxt, memorySpace);
60+
TypeBuilderWithInferredContext<(ins
61+
"MemorySpaceAttrInterface":$memorySpace), [{
62+
return $_get(memorySpace.getContext(), memorySpace);
6263
}]>
6364
];
64-
let skipDefaultBuilders = 1;
6565
}
6666

6767
//===----------------------------------------------------------------------===//

‎mlir/lib/Dialect/Ptr/IR/PtrTypes.cpp

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ constexpr const static unsigned kDefaultPointerAlignmentBits = 8;
2828
/// Searches the data layout for the pointer spec, returns nullptr if it is not
2929
/// found.
3030
static SpecAttr getPointerSpec(DataLayoutEntryListRef params, PtrType type,
31-
Attribute defaultMemorySpace) {
31+
MemorySpaceAttrInterface defaultMemorySpace) {
3232
for (DataLayoutEntryInterface entry : params) {
3333
if (!entry.isTypeEntry())
3434
continue;
@@ -38,9 +38,11 @@ static SpecAttr getPointerSpec(DataLayoutEntryListRef params, PtrType type,
3838
return spec;
3939
}
4040
}
41-
// If not found, and this is the pointer to the default memory space, assume
42-
// 64-bit pointers.
43-
if (type.getMemorySpace() == defaultMemorySpace)
41+
// If not found, and this is the pointer to the default memory space or if
42+
// `defaultMemorySpace` is null, assume 64-bit pointers. `defaultMemorySpace`
43+
// might be null if the data layout doesn't define the default memory space.
44+
if (type.getMemorySpace() == defaultMemorySpace ||
45+
defaultMemorySpace == nullptr)
4446
return SpecAttr::get(type.getContext(), kDefaultPointerSizeBits,
4547
kDefaultPointerAlignmentBits,
4648
kDefaultPointerAlignmentBits, kDefaultPointerSizeBits);
@@ -93,44 +95,47 @@ bool PtrType::areCompatible(DataLayoutEntryListRef oldLayout,
9395

9496
uint64_t PtrType::getABIAlignment(const DataLayout &dataLayout,
9597
DataLayoutEntryListRef params) const {
96-
Attribute defaultMemorySpace = dataLayout.getDefaultMemorySpace();
98+
auto defaultMemorySpace = llvm::cast_if_present<MemorySpaceAttrInterface>(
99+
dataLayout.getDefaultMemorySpace());
97100
if (SpecAttr spec = getPointerSpec(params, *this, defaultMemorySpace))
98101
return spec.getAbi() / kBitsInByte;
99102

100-
return dataLayout.getTypeABIAlignment(get(getContext(), defaultMemorySpace));
103+
return dataLayout.getTypeABIAlignment(get(defaultMemorySpace));
101104
}
102105

103106
std::optional<uint64_t>
104107
PtrType::getIndexBitwidth(const DataLayout &dataLayout,
105108
DataLayoutEntryListRef params) const {
106-
Attribute defaultMemorySpace = dataLayout.getDefaultMemorySpace();
109+
auto defaultMemorySpace = llvm::cast_if_present<MemorySpaceAttrInterface>(
110+
dataLayout.getDefaultMemorySpace());
107111
if (SpecAttr spec = getPointerSpec(params, *this, defaultMemorySpace)) {
108112
return spec.getIndex() == SpecAttr::kOptionalSpecValue ? spec.getSize()
109113
: spec.getIndex();
110114
}
111115

112-
return dataLayout.getTypeIndexBitwidth(get(getContext(), defaultMemorySpace));
116+
return dataLayout.getTypeIndexBitwidth(get(defaultMemorySpace));
113117
}
114118

115119
llvm::TypeSize PtrType::getTypeSizeInBits(const DataLayout &dataLayout,
116120
DataLayoutEntryListRef params) const {
117-
Attribute defaultMemorySpace = dataLayout.getDefaultMemorySpace();
121+
auto defaultMemorySpace = llvm::cast_if_present<MemorySpaceAttrInterface>(
122+
dataLayout.getDefaultMemorySpace());
118123
if (SpecAttr spec = getPointerSpec(params, *this, defaultMemorySpace))
119124
return llvm::TypeSize::getFixed(spec.getSize());
120125

121126
// For other memory spaces, use the size of the pointer to the default memory
122127
// space.
123-
return dataLayout.getTypeSizeInBits(get(getContext(), defaultMemorySpace));
128+
return dataLayout.getTypeSizeInBits(get(defaultMemorySpace));
124129
}
125130

126131
uint64_t PtrType::getPreferredAlignment(const DataLayout &dataLayout,
127132
DataLayoutEntryListRef params) const {
128-
Attribute defaultMemorySpace = dataLayout.getDefaultMemorySpace();
133+
auto defaultMemorySpace = llvm::cast_if_present<MemorySpaceAttrInterface>(
134+
dataLayout.getDefaultMemorySpace());
129135
if (SpecAttr spec = getPointerSpec(params, *this, defaultMemorySpace))
130136
return spec.getPreferred() / kBitsInByte;
131137

132-
return dataLayout.getTypePreferredAlignment(
133-
get(getContext(), defaultMemorySpace));
138+
return dataLayout.getTypePreferredAlignment(get(defaultMemorySpace));
134139
}
135140

136141
LogicalResult PtrType::verifyEntries(DataLayoutEntryListRef entries,

‎mlir/test/Dialect/Ptr/layout.mlir

Lines changed: 26 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -4,55 +4,55 @@ module attributes { dlti.dl_spec = #dlti.dl_spec<
44
#dlti.dl_entry<!ptr.ptr<#test.const_memory_space>, #ptr.spec<size = 32, abi = 32, preferred = 64>>,
55
#dlti.dl_entry<!ptr.ptr<#test.const_memory_space<5>>,#ptr.spec<size = 64, abi = 64, preferred = 64>>,
66
#dlti.dl_entry<!ptr.ptr<#test.const_memory_space<4>>, #ptr.spec<size = 32, abi = 64, preferred = 64, index = 24>>,
7-
#dlti.dl_entry<"dlti.default_memory_space", 7 : ui64>,
8-
#dlti.dl_entry<"dlti.alloca_memory_space", 5 : ui64>,
9-
#dlti.dl_entry<"dlti.global_memory_space", 2 : ui64>,
10-
#dlti.dl_entry<"dlti.program_memory_space", 3 : ui64>,
7+
#dlti.dl_entry<"dlti.default_memory_space", #test.const_memory_space<7>>,
8+
#dlti.dl_entry<"dlti.alloca_memory_space", #test.const_memory_space<5>>,
9+
#dlti.dl_entry<"dlti.global_memory_space", #test.const_memory_space<2>>,
10+
#dlti.dl_entry<"dlti.program_memory_space", #test.const_memory_space<3>>,
1111
#dlti.dl_entry<"dlti.stack_alignment", 128 : i64>
1212
>} {
1313
// CHECK-LABEL: @spec
1414
func.func @spec() {
1515
// CHECK: alignment = 4
16-
// CHECK: alloca_memory_space = 5
16+
// CHECK: alloca_memory_space = #test.const_memory_space<5>
1717
// CHECK: bitsize = 32
18-
// CHECK: default_memory_space = 7
19-
// CHECK: global_memory_space = 2
18+
// CHECK: default_memory_space = #test.const_memory_space<7>
19+
// CHECK: global_memory_space = #test.const_memory_space<2>
2020
// CHECK: index = 32
2121
// CHECK: preferred = 8
22-
// CHECK: program_memory_space = 3
22+
// CHECK: program_memory_space = #test.const_memory_space<3>
2323
// CHECK: size = 4
2424
// CHECK: stack_alignment = 128
2525
"test.data_layout_query"() : () -> !ptr.ptr<#test.const_memory_space>
26-
// CHECK: alignment = 4
27-
// CHECK: alloca_memory_space = 5
26+
// CHECK: alignment = 1
27+
// CHECK: alloca_memory_space = #test.const_memory_space<5>
2828
// CHECK: bitsize = 64
29-
// CHECK: default_memory_space = 7
30-
// CHECK: global_memory_space = 2
29+
// CHECK: default_memory_space = #test.const_memory_space<7>
30+
// CHECK: global_memory_space = #test.const_memory_space<2>
3131
// CHECK: index = 64
3232
// CHECK: preferred = 1
33-
// CHECK: program_memory_space = 3
33+
// CHECK: program_memory_space = #test.const_memory_space<3>
3434
// CHECK: size = 8
3535
// CHECK: stack_alignment = 128
3636
"test.data_layout_query"() : () -> !ptr.ptr<#test.const_memory_space<3>>
3737
// CHECK: alignment = 8
38-
// CHECK: alloca_memory_space = 5
38+
// CHECK: alloca_memory_space = #test.const_memory_space<5>
3939
// CHECK: bitsize = 64
40-
// CHECK: default_memory_space = 7
41-
// CHECK: global_memory_space = 2
40+
// CHECK: default_memory_space = #test.const_memory_space<7>
41+
// CHECK: global_memory_space = #test.const_memory_space<2>
4242
// CHECK: index = 64
4343
// CHECK: preferred = 8
44-
// CHECK: program_memory_space = 3
44+
// CHECK: program_memory_space = #test.const_memory_space<3>
4545
// CHECK: size = 8
4646
// CHECK: stack_alignment = 128
4747
"test.data_layout_query"() : () -> !ptr.ptr<#test.const_memory_space<5>>
4848
// CHECK: alignment = 8
49-
// CHECK: alloca_memory_space = 5
49+
// CHECK: alloca_memory_space = #test.const_memory_space<5>
5050
// CHECK: bitsize = 32
51-
// CHECK: default_memory_space = 7
52-
// CHECK: global_memory_space = 2
51+
// CHECK: default_memory_space = #test.const_memory_space<7>
52+
// CHECK: global_memory_space = #test.const_memory_space<2>
5353
// CHECK: index = 24
5454
// CHECK: preferred = 8
55-
// CHECK: program_memory_space = 3
55+
// CHECK: program_memory_space = #test.const_memory_space<3>
5656
// CHECK: size = 4
5757
// CHECK: stack_alignment = 128
5858
"test.data_layout_query"() : () -> !ptr.ptr<#test.const_memory_space<4>>
@@ -63,8 +63,8 @@ module attributes { dlti.dl_spec = #dlti.dl_spec<
6363
// -----
6464

6565
module attributes { dlti.dl_spec = #dlti.dl_spec<
66-
#dlti.dl_entry<!ptr.ptr<1 : ui64>, #ptr.spec<size = 32, abi = 32, preferred = 32>>,
67-
#dlti.dl_entry<"dlti.default_memory_space", 1 : ui64>
66+
#dlti.dl_entry<!ptr.ptr<#test.const_memory_space>, #ptr.spec<size = 32, abi = 32, preferred = 32>>,
67+
#dlti.dl_entry<"dlti.default_memory_space", #test.const_memory_space>
6868
>} {
6969
// CHECK-LABEL: @default_memory_space
7070
func.func @default_memory_space() {
@@ -73,19 +73,19 @@ module attributes { dlti.dl_spec = #dlti.dl_spec<
7373
// CHECK: index = 32
7474
// CHECK: preferred = 4
7575
// CHECK: size = 4
76-
"test.data_layout_query"() : () -> !ptr.ptr
76+
"test.data_layout_query"() : () -> !ptr.ptr<#test.const_memory_space>
7777
// CHECK: alignment = 4
7878
// CHECK: bitsize = 32
7979
// CHECK: index = 32
8080
// CHECK: preferred = 4
8181
// CHECK: size = 4
82-
"test.data_layout_query"() : () -> !ptr.ptr<1>
82+
"test.data_layout_query"() : () -> !ptr.ptr<#test.const_memory_space<1>>
8383
// CHECK: alignment = 4
8484
// CHECK: bitsize = 32
8585
// CHECK: index = 32
8686
// CHECK: preferred = 4
8787
// CHECK: size = 4
88-
"test.data_layout_query"() : () -> !ptr.ptr<2>
88+
"test.data_layout_query"() : () -> !ptr.ptr<#test.const_memory_space<2>>
8989
return
9090
}
9191
}

‎mlir/test/lib/Dialect/Test/TestAttributes.cpp

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -331,11 +331,6 @@ TestOpAsmAttrInterfaceAttr::getAlias(::llvm::raw_ostream &os) const {
331331
// TestConstMemorySpaceAttr
332332
//===----------------------------------------------------------------------===//
333333

334-
ptr::MemorySpaceAttrInterface
335-
TestConstMemorySpaceAttr::getDefaultMemorySpace() const {
336-
return TestConstMemorySpaceAttr::get(getContext(), 0);
337-
}
338-
339334
LogicalResult TestConstMemorySpaceAttr::isValidLoad(
340335
Type type, mlir::ptr::AtomicOrdering ordering, IntegerAttr alignment,
341336
function_ref<InFlightDiagnostic()> emitError) const {

0 commit comments

Comments
 (0)