Page MenuHomePhabricator

[mlir] Add asserts when changing various MLIRContext configurations
ClosedPublic

Authored by rriddle on Oct 18 2021, 10:50 AM.

Details

Summary

This helps to prevent tsan failures when users inadvertantly mutate the
context in a non-safe way.

Diff Detail

Event Timeline

rriddle created this revision.Oct 18 2021, 10:50 AM
rriddle requested review of this revision.Oct 18 2021, 10:50 AM
This revision is now accepted and ready to land.Oct 18 2021, 10:57 AM
rriddle updated this revision to Diff 401032.Jan 18 2022, 4:17 PM
rriddle edited the summary of this revision. (Show Details)
rriddle updated this revision to Diff 408143.Feb 11 2022, 7:29 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 15 2022, 9:49 PM
mlir/lib/IR/MLIRContext.cpp
361

I have a general question here on the registration requirements with nested passes.

Is there a good rule of thumb for dependent dialects, external models and dynamically injected ops mixed with nested passes?
It seems that a mechanism to declare dependent passes would be a good QoL improvement: it does not seem manageable for nested pass writers to have to manually add registration for everything in dependent passes.

Am I missing something?

mehdi_amini added inline comments.Apr 21 2022, 1:12 PM
mlir/lib/IR/MLIRContext.cpp
361

Dependent dialects is supposed to be handle recursively, which admittedly can be tricky sometimes. I think the IREE HAL handles it properly as an example: https://github.com/google/iree/blob/main/iree/compiler/Dialect/HAL/Transforms/TranslateExecutables.cpp#L46-L53

External models are supposed to be injected by IR producers, if this is part of a nested pass, the pass that nest this can handle this in getDependentDialects() as well.