Skip to content

Do not create ADDR(HWI/SIMD) in the inliner - #74276

Merged
AndyAyersMS merged 4 commits into
dotnet:mainfrom
SingleAccretion:No-Addr-Node-Inlining-Upstream
Sep 7, 2022
Merged

AndyAyersMS merged 4 commits into
dotnet:mainfrom
SingleAccretion:No-Addr-Node-Inlining-Upstream

Conversation

@SingleAccretion

@SingleAccretion SingleAccretion commented Aug 19, 2022 •

Copy link
Copy Markdown
Contributor

When inserting return expressions in the inliner, we do some IR munging to maintain the invariant that multi-reg nodes are only used by multi-reg stores. This procedure was creating some illegal ADDRs, which only "worked out" because block morphing would later always discard them. This change avoids that by creating simple stores from the beginning.

We're expecting sizeable positive diffs due to unblocking a number of forward substitutions that would otherwise fail on the "multi-reg state must match" checks.

Diffs - with some regressions due to the usual consequences of more propagations.

@ghost ghost added community-contribution Indicates that the PR has been added by a community member area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI labels Aug 19, 2022
@ghost

ghost commented Aug 19, 2022

Copy link
Copy Markdown

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

Issue Details

When inserting return expressions in the inliner, we do some IR munging to maintain the invariant that multi-reg nodes are only used by multi-reg stores. This procedure was creating some illegal ADDR, which only "worked out" because block morphing would later always discard them. This change avoids that by creating simple stores from the beginning.

We're expecting sizeable positive diffs due to unblocking a number of forward substitutions that would otherwise fail on the "multi-reg state must match" checks.

Author: SingleAccretion
Assignees: -
Labels:

area-CodeGen-coreclr, community-contribution

Milestone: -

@JulieLeeMSFT JulieLeeMSFT added this to the 8.0.0 milestone Aug 19, 2022
@SingleAccretion
SingleAccretion force-pushed the No-Addr-Node-Inlining-Upstream branch from f404c3a to e574525 Compare August 22, 2022 10:04
@SingleAccretion
SingleAccretion marked this pull request as ready for review August 22, 2022 16:40
@SingleAccretion

Copy link
Copy Markdown
Contributor Author

@dotnet/jit-contrib

@SingleAccretion
SingleAccretion force-pushed the No-Addr-Node-Inlining-Upstream branch from e574525 to 90d6797 Compare August 23, 2022 13:35
Integer zero works better with ASG parents (they turn into init blks).
@SingleAccretion
SingleAccretion force-pushed the No-Addr-Node-Inlining-Upstream branch from 90d6797 to 73e70fe Compare August 23, 2022 13:38
@AndyAyersMS

Copy link
Copy Markdown
Member

@EgorBo @jakobbotsch can one of you take a look at this?

@AndyAyersMS

Copy link
Copy Markdown
Member

Let's bounce this one.

@AndyAyersMS AndyAyersMS closed this Sep 1, 2022
@AndyAyersMS AndyAyersMS reopened this Sep 1, 2022
@AndyAyersMS
AndyAyersMS merged commit ebd80ad into dotnet:main Sep 7, 2022
@AndyAyersMS

Copy link
Copy Markdown
Member

Thanks again for your contribution!

@SingleAccretion
SingleAccretion deleted the No-Addr-Node-Inlining-Upstream branch September 12, 2022 13:16
@AndyAyersMS

AndyAyersMS commented Sep 14, 2022 •

Copy link
Copy Markdown
Member

@ghost ghost locked as resolved and limited conversation to collaborators Oct 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants