llvm / llvm/circt

[Ibis] Update direct uses of replaceAllUsesWith.

Open
#6,795 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Kanagawa
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

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.