[Ibis] Update direct uses of replaceAllUsesWith.
Nobody has claimed this yet.
- Dominant language
- C++
- Stars
- 2.2k
- Forks
- 524
- Avg merge
- 3d 2h
- Merged PRs (30d)
- 46
Description
In https://github.com/llvm/circt/issues/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
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by locating the Ibis pass uses of RewriterBase::replaceAllUsesWith and read the related DialectConversion behavior described in issue 6793. Compare those uses with ConversionPatternRewriter::replaceOp and its supported APIs, checking the assumptions that caused undefined behavior. Done means the direct replacements use a supported API, or the appropriate ConversionPatternRewriter method when available, without triggering the DialectConversion assertions.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- compilers
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100