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

[AutoBump] Merge with fixes of f1e0657d (Jun 25, requires downstream fixes) (84) #348

Merged
merged 2 commits into from
Sep 16, 2024

Conversation

mgehre-amd
Copy link
Collaborator

@mgehre-amd mgehre-amd commented Sep 13, 2024

Causes Assertion container && "region is not attached to a container"' failed` downstream

matthias-springer and others added 2 commits June 25, 2024 08:43
…rguments (llvm#96207)

This commit simplifies the handling of dropped arguments and updates
some dialect conversion documentation that is outdated.

When converting a block signature, a `BlockTypeConversionRewrite` object
and potentially multiple `ReplaceBlockArgRewrite` are created. During
the "commit" phase, uses of the old block arguments are replaced with
the new block arguments, but the old implementation was written in an
inconsistent way: some block arguments were replaced in
`BlockTypeConversionRewrite::commit` and some were replaced in
`ReplaceBlockArgRewrite::commit`. The new
`BlockTypeConversionRewrite::commit` implementation is much simpler and
no longer modifies any IR; that is done only in `ReplaceBlockArgRewrite`
now. The `ConvertedArgInfo` data structure is no longer needed.

To that end, materializations of dropped arguments are now built in
`applySignatureConversion` instead of `materializeLiveConversions`; the
latter function no longer has to deal with dropped arguments.

Other minor improvements:
- Improve variable name: `origOutputType` -> `origArgType`. Add an
assertion to check that this field is only used for argument
materializations.
- Add more comments to `applySignatureConversion`.

Note: Error messages around failed materializations for dropped basic
block arguments changed slightly. That is because those materializations
are now built in `legalizeUnresolvedMaterialization` instead of
`legalizeConvertedArgumentTypes`.

This commit is in preparation of decoupling argument/source/target
materializations from the dialect conversion.
@mgehre-amd mgehre-amd changed the title [AutoBump] Merge with fixes of f1e0657d (Jun 25) (84) [AutoBump] Merge with fixes of f1e0657d (Jun 25, requires downstream fixes) (84) Sep 13, 2024
Base automatically changed from bump_to_62d44fbd to feature/fused-ops September 16, 2024 13:43
@mgehre-amd mgehre-amd merged commit 16c1789 into feature/fused-ops Sep 16, 2024
4 checks passed
@mgehre-amd mgehre-amd deleted the bump_to_f1e0657d branch September 16, 2024 13:43
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.

3 participants