Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PipelineToHW] Fix PipelineToHW TSAN issue #5817

Merged
merged 1 commit into from
Aug 9, 2023

Conversation

mortbopet
Copy link
Contributor

@mortbopet mortbopet commented Aug 9, 2023

Fixing a TSAN issue due to concurrent additions (and removals) to the top-level mlir::ModuleOp symbol table, whilst the pass is running at a hw::HWModuleOp granularity.

fixes #5815

Fixing a TSAN issue due to concurrent additions (and removals) to the top-level mlir::ModuleOp symbol table, whilst the pass is running at a hw::HWModuleOp granularity.
@mortbopet mortbopet changed the title [PipelineToHW] Fix PipelineToHW TSAN issue #5815 [PipelineToHW] Fix PipelineToHW TSAN issue Aug 9, 2023
// here by allowing this pass to run on a per-hwmodule/whatever container the
// pipeline is nested within-granularity. However, this conversion adds (and removes)
// new modules to the top-level mlir::ModuleOp scope, which technically violates
// hw::HWModuleLike's IsolatedFromAbove (and thus has previously caused
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW don't think whether it's IsolatedFromAbove is relevant, it's that passes cannot change the containing operation (or even the current one in ways that's visible/possibly-breaking to other operations, such as adding/removing ports of a hw.module from a pass running on hw.module's).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

passes cannot change the containing operation

I see IsolatedFromAbove as a trait inherently linked to the passmanager, and how passes are scheduled. IsolatedFromAbove allows multi-threaded execution, which in turn is what "allows" these errors to happen.

Copy link
Contributor

@dtzSiFive dtzSiFive left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@mortbopet mortbopet merged commit ef90b61 into main Aug 9, 2023
5 checks passed
@darthscsi darthscsi deleted the dev/mpetersen/pipelinetohw_fix branch June 4, 2024 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[PipelineToHW] Conversion/PipelineToHW/test_outlined.mlir failure
2 participants