Skip to content

Commit 2bee246

Browse files
authored
[mlir][bugfix] Fix erroneous condition in getEffectsOnResource (#133638)
This patch corrects an invalid condition in `getEffectsOnResource` used to identify relevant "resources": ```cpp return it.getResource() != resource; ``` The current implementation assumes that only one instance of each resource will exist, so comparing raw pointers is both safe and sufficient. This assumption stems from constructs like: ```cpp static DerivedResource *get() { static DerivedResource instance; return &instance; } ``` i.e., resource instances returned via static singleton methods. However, as discussed in * #129216, this assumption breaks in practice — notably on macOS (Apple Silicon) when built with: * `-DBUILD_SHARED_LIBS=On`. In such cases, multiple instances of the same logical resource may exist across shared library boundaries, leading to incorrect behavior and causing failures in tests like: * test/Dialect/Transform/check-use-after-free.mlir This patch replaces the pointer comparison with a comparison based on resource identity: ```cpp return it.getResource()->getResourceID() != resource->getResourceID(); ``` This approach aligns better with the intent of `getEffectsOnResource`, which is to: ```cpp /// Collect all of the effect instances that operate on the provided /// resource (...) ``` Fixes #129216
1 parent df9e5ae commit 2bee246

File tree

1 file changed

+1
-1
lines changed

1 file changed

+1
-1
lines changed

mlir/include/mlir/Interfaces/SideEffectInterfaceBase.td

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ class EffectOpInterfaceBase<string name, string baseEffect>
140140
}] # baseEffect # [{>> & effects) {
141141
getEffects(effects);
142142
::llvm::erase_if(effects, [&](auto &it) {
143-
return it.getResource() != resource;
143+
return it.getResource()->getResourceID() != resource->getResourceID();
144144
});
145145
}
146146
}];

0 commit comments

Comments
 (0)