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

[Ibis] Update direct uses of replaceAllUsesWith. #6795

Open
mikeurbach opened this issue Mar 7, 2024 · 0 comments
Open

[Ibis] Update direct uses of replaceAllUsesWith. #6795

mikeurbach opened this issue Mar 7, 2024 · 0 comments
Labels

Comments

@mikeurbach
Copy link
Contributor

In #6793, we faced an issue where newly added asserts in DialectConversion were triggered by uses of rewriter.replaceAllUsesWith. This is because RewriterBase::replaceAllUsesWith is not currently supported by DialectConversion.

To work around this for now, we updated the passes to do this RAUW directly, until ConversionPatternRewriter properly supports replaceAllUsesWith. Since DialectConversion already doesn't intercept or do any bookkeeping for RewriterBase::replaceAllUsesWith, this is no less safe than the previous usage of RewriterBase::replaceAllUsesWith.

If possible, we should find a way to use the supported APIs of ConversionPatternRewriter, like replaceOp. I tried making the trivial change, but found that it violated other assumptions of the passes in question, and led to undefined behavior within the DialectConversion framework.

If these uses can only be implemented with replaceAllUsesWith, we should update these to use ConversionPatternRewriter::replaceAllUsesWith once that is available.

For more background, see the Discord discussion around https://discord.com/channels/636084430946959380/642426447167881246/1215098183739252806.

FYI @mortbopet @teqdruid

@mikeurbach mikeurbach added the Ibis label Mar 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant