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

Fix propagation of basic block flags for inlinee return expressions. #37840

Merged
merged 1 commit into from
Jun 15, 2020

Conversation

erozenfeld
Copy link
Member

#37335 introduced a new more precise way of propagating basic
block flags from inlinee return expressions. The assumption there
was that fgUpdateInlineReturnExpressionPlaceHolder is the only
place where GT_RET_EXPR's are replaced with the actual nodes
from inlinees. That assumption was incorrect and this change fixes
the remaining places.

Fixes #37574.

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jun 13, 2020
@erozenfeld
Copy link
Member Author

No diffs in x64 pmi framewors and benchmarks.

@erozenfeld
Copy link
Member Author

@dotnet/jit-contrib @AndyAyersMS PTAL

Copy link
Member

@AndyAyersMS AndyAyersMS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I owe you an apology -- when I looked at your first fix for this I thought to ask if you handled the ret expr tunneling, but didn't. Sorry about that.

GenTree* argNode; // caller node for this argument
GenTree* argBashTmpNode; // tmp node created, if it may be replaced with actual arg
unsigned argTmpNum; // the argument tmp number
unsigned __int64 bbFlags; // basic block flags that need to be added when replacing GT_RET_EXPR
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I'd place this field first to minimize the amount of padding we might end up with here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, done.

@erozenfeld erozenfeld force-pushed the Fix37574 branch 2 times, most recently from 94b2c4e to f0128d0 Compare June 13, 2020 06:22
block flags from inlinee return expressions. The assumption there
was that `fgUpdateInlineReturnExpressionPlaceHolder` is the only
place where `GT_RET_EXPR`'s are replaced with the actual nodes
from inlinees. That assumption was incorrect and this change fixes
the remaining places.

Fixes dotnet#37574.
@erozenfeld erozenfeld merged commit a96a64e into dotnet:master Jun 15, 2020
erozenfeld added a commit to erozenfeld/runtime that referenced this pull request Jun 22, 2020
This is a fllow-up to dotnet#37335 and dotnet#37840.

When an inline fails we replace `GT_RET_EXPR`with the
original `GT_CALL` node. `GT_RET_EXPR`may end up in
a basic block other than the original `GT_CALL` so we need
to propagate basic block flags.

Fixes dotnet#36588.
erozenfeld added a commit that referenced this pull request Jun 23, 2020
This is a fllow-up to #37335 and #37840.

When an inline fails we replace `GT_RET_EXPR`with the
original `GT_CALL` node. `GT_RET_EXPR`may end up in
a basic block other than the original `GT_CALL` so we need
to propagate basic block flags.

Fixes #36588.
@ghost ghost locked as resolved and limited conversation to collaborators Dec 8, 2020
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
Projects
None yet
3 participants