Skip to content

Commit 084e2b5

Browse files
authored
[MLIR][Interfaces] Change MemorySlotInterface to use OpBuilder (#91341)
This commit changes the `MemorySlotInterface` back to using `OpBuilder` instead of a rewriter. This was originally introduced in https://reviews.llvm.org/D150432 but it was shown that patterns are a bad idea for both Mem2Reg and SROA. Mem2Reg suffers from the usage of a rewriter due to being forced to create new basic blocks. This is an issue, as it leads to the invalidation of the dominance information, which can be expensive to recompute.
1 parent 3e82442 commit 084e2b5

File tree

7 files changed

+216
-267
lines changed

7 files changed

+216
-267
lines changed

mlir/include/mlir/Interfaces/MemorySlotInterfaces.td

Lines changed: 27 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -40,42 +40,40 @@ def PromotableAllocationOpInterface
4040
Provides the default Value of this memory slot. The provided Value
4141
will be used as the reaching definition of loads done before any store.
4242
This Value must outlive the promotion and dominate all the uses of this
43-
slot's pointer. The provided rewriter can be used to create the default
43+
slot's pointer. The provided builder can be used to create the default
4444
value on the fly.
4545

46-
The rewriter is located at the beginning of the block where the slot
47-
pointer is defined. All IR mutations must happen through the rewriter.
46+
The builder is located at the beginning of the block where the slot
47+
pointer is defined.
4848
}], "::mlir::Value", "getDefaultValue",
4949
(ins
5050
"const ::mlir::MemorySlot &":$slot,
51-
"::mlir::RewriterBase &":$rewriter)
51+
"::mlir::OpBuilder &":$builder)
5252
>,
5353
InterfaceMethod<[{
5454
Hook triggered for every new block argument added to a block.
5555
This will only be called for slots declared by this operation.
5656

57-
The rewriter is located at the beginning of the block on call. All IR
58-
mutations must happen through the rewriter.
57+
The builder is located at the beginning of the block on call. All IR
58+
mutations must happen through the builder.
5959
}],
6060
"void", "handleBlockArgument",
6161
(ins
6262
"const ::mlir::MemorySlot &":$slot,
6363
"::mlir::BlockArgument":$argument,
64-
"::mlir::RewriterBase &":$rewriter
64+
"::mlir::OpBuilder &":$builder
6565
)
6666
>,
6767
InterfaceMethod<[{
6868
Hook triggered once the promotion of a slot is complete. This can
6969
also clean up the created default value if necessary.
7070
This will only be called for slots declared by this operation.
71-
72-
All IR mutations must happen through the rewriter.
7371
}],
7472
"void", "handlePromotionComplete",
7573
(ins
7674
"const ::mlir::MemorySlot &":$slot,
7775
"::mlir::Value":$defaultValue,
78-
"::mlir::RewriterBase &":$rewriter)
76+
"::mlir::OpBuilder &":$builder)
7977
>,
8078
];
8179
}
@@ -119,15 +117,14 @@ def PromotableMemOpInterface : OpInterface<"PromotableMemOpInterface"> {
119117
The returned value must dominate all operations dominated by the storing
120118
operation.
121119

122-
If IR must be mutated to extract a concrete value being stored, mutation
123-
must happen through the provided rewriter. The rewriter is located
124-
immediately after the memory operation on call. No IR deletion is
125-
allowed in this method. IR mutations must not introduce new uses of the
126-
memory slot. Existing control flow must not be modified.
120+
The builder is located immediately after the memory operation on call.
121+
No IR deletion is allowed in this method. IR mutations must not
122+
introduce new uses of the memory slot. Existing control flow must not
123+
be modified.
127124
}],
128125
"::mlir::Value", "getStored",
129126
(ins "const ::mlir::MemorySlot &":$slot,
130-
"::mlir::RewriterBase &":$rewriter,
127+
"::mlir::OpBuilder &":$builder,
131128
"::mlir::Value":$reachingDef,
132129
"const ::mlir::DataLayout &":$dataLayout)
133130
>,
@@ -166,14 +163,13 @@ def PromotableMemOpInterface : OpInterface<"PromotableMemOpInterface"> {
166163
have been done at the point of calling this method, but it will be done
167164
eventually.
168165

169-
The rewriter is located after the promotable operation on call. All IR
170-
mutations must happen through the rewriter.
166+
The builder is located after the promotable operation on call.
171167
}],
172168
"::mlir::DeletionKind",
173169
"removeBlockingUses",
174170
(ins "const ::mlir::MemorySlot &":$slot,
175171
"const ::llvm::SmallPtrSetImpl<mlir::OpOperand *> &":$blockingUses,
176-
"::mlir::RewriterBase &":$rewriter,
172+
"::mlir::OpBuilder &":$builder,
177173
"::mlir::Value":$reachingDefinition,
178174
"const ::mlir::DataLayout &":$dataLayout)
179175
>,
@@ -224,13 +220,12 @@ def PromotableOpInterface : OpInterface<"PromotableOpInterface"> {
224220
have been done at the point of calling this method, but it will be done
225221
eventually.
226222

227-
The rewriter is located after the promotable operation on call. All IR
228-
mutations must happen through the rewriter.
223+
The builder is located after the promotable operation on call.
229224
}],
230225
"::mlir::DeletionKind",
231226
"removeBlockingUses",
232227
(ins "const ::llvm::SmallPtrSetImpl<mlir::OpOperand *> &":$blockingUses,
233-
"::mlir::RewriterBase &":$rewriter)
228+
"::mlir::OpBuilder &":$builder)
234229
>,
235230
InterfaceMethod<[{
236231
This method allows the promoted operation to visit the SSA values used
@@ -254,13 +249,12 @@ def PromotableOpInterface : OpInterface<"PromotableOpInterface"> {
254249
scheduled for removal and if `requiresReplacedValues` returned
255250
true.
256251

257-
The rewriter is located after the promotable operation on call. All IR
258-
mutations must happen through the rewriter. During the transformation,
259-
*no operation should be deleted*.
252+
The builder is located after the promotable operation on call. During
253+
the transformation, *no operation should be deleted*.
260254
}],
261255
"void", "visitReplacedValues",
262256
(ins "::llvm::ArrayRef<std::pair<::mlir::Operation*, ::mlir::Value>>":$mutatedDefs,
263-
"::mlir::RewriterBase &":$rewriter), [{}], [{ return; }]
257+
"::mlir::OpBuilder &":$builder), [{}], [{ return; }]
264258
>,
265259
];
266260
}
@@ -293,25 +287,23 @@ def DestructurableAllocationOpInterface
293287
at the end of this call. Only generates subslots for the indices found in
294288
`usedIndices` since all other subslots are unused.
295289

296-
The rewriter is located at the beginning of the block where the slot
297-
pointer is defined. All IR mutations must happen through the rewriter.
290+
The builder is located at the beginning of the block where the slot
291+
pointer is defined.
298292
}],
299293
"::llvm::DenseMap<::mlir::Attribute, ::mlir::MemorySlot>",
300294
"destructure",
301295
(ins "const ::mlir::DestructurableMemorySlot &":$slot,
302296
"const ::llvm::SmallPtrSetImpl<::mlir::Attribute> &":$usedIndices,
303-
"::mlir::RewriterBase &":$rewriter)
297+
"::mlir::OpBuilder &":$builder)
304298
>,
305299
InterfaceMethod<[{
306300
Hook triggered once the destructuring of a slot is complete, meaning the
307301
original slot is no longer being refered to and could be deleted.
308302
This will only be called for slots declared by this operation.
309-
310-
All IR mutations must happen through the rewriter.
311303
}],
312304
"void", "handleDestructuringComplete",
313305
(ins "const ::mlir::DestructurableMemorySlot &":$slot,
314-
"::mlir::RewriterBase &":$rewriter)
306+
"::mlir::OpBuilder &":$builder)
315307
>,
316308
];
317309
}
@@ -376,15 +368,14 @@ def DestructurableAccessorOpInterface
376368
Rewires the use of a slot to the generated subslots, without deleting
377369
any operation. Returns whether the accessor should be deleted.
378370

379-
All IR mutations must happen through the rewriter. Deletion of
380-
operations is not allowed, only the accessor can be scheduled for
381-
deletion by returning the appropriate value.
371+
Deletion of operations is not allowed, only the accessor can be
372+
scheduled for deletion by returning the appropriate value.
382373
}],
383374
"::mlir::DeletionKind",
384375
"rewire",
385376
(ins "const ::mlir::DestructurableMemorySlot &":$slot,
386377
"::llvm::DenseMap<::mlir::Attribute, ::mlir::MemorySlot> &":$subslots,
387-
"::mlir::RewriterBase &":$rewriter,
378+
"::mlir::OpBuilder &":$builder,
388379
"const ::mlir::DataLayout &":$dataLayout)
389380
>
390381
];

mlir/include/mlir/Transforms/Mem2Reg.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ struct Mem2RegStatistics {
2727
/// at least one memory slot was promoted.
2828
LogicalResult
2929
tryToPromoteMemorySlots(ArrayRef<PromotableAllocationOpInterface> allocators,
30-
RewriterBase &rewriter, const DataLayout &dataLayout,
30+
OpBuilder &builder, const DataLayout &dataLayout,
3131
Mem2RegStatistics statistics = {});
3232

3333
} // namespace mlir

mlir/include/mlir/Transforms/SROA.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ struct SROAStatistics {
3131
/// failure if no slot was destructured.
3232
LogicalResult tryToDestructureMemorySlots(
3333
ArrayRef<DestructurableAllocationOpInterface> allocators,
34-
RewriterBase &rewriter, const DataLayout &dataLayout,
34+
OpBuilder &builder, const DataLayout &dataLayout,
3535
SROAStatistics statistics = {});
3636

3737
} // namespace mlir

0 commit comments

Comments
 (0)