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

Imply NoInlining when only NoOptimization is set #111308

Merged
merged 6 commits into from
Jan 13, 2025

Conversation

EgorBo
Copy link
Member

@EgorBo EgorBo commented Jan 11, 2025

Closes #111298

Alternatively, we can only change IsMiNoInlining's implementation to check either flag.

@jkotas
Copy link
Member

jkotas commented Jan 12, 2025

Do we need the same change in AOT compilers?

@EgorBo
Copy link
Member Author

EgorBo commented Jan 12, 2025

Do we need the same change in AOT compilers?

Oops, changed for AOT too.

// to mean "I want to always see this method in stacktrace")
if (caller.IsNoInlining)
if (caller.IsNoInlining || caller.IsNoOptimization)
Copy link
Member

Choose a reason for hiding this comment

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

Is it necessary to check for NoOptimization here? I would expect that the JIT disables tailcall optimizations when the method is marked NoOptimization.

Copy link
Member Author

@EgorBo EgorBo Jan 13, 2025

Choose a reason for hiding this comment

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

@jkotas um.. jit is not aware that a call has NoOptimization on it so it does emit tail calls for it, example

ah, ignore me. it's about calls inside the NoOptimization method. So yes, you're right - it's redundant 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.

Removed

@EgorBo EgorBo merged commit b6be804 into dotnet:main Jan 13, 2025
87 of 91 checks passed
@EgorBo EgorBo deleted the dont-inline-nooptimization branch January 13, 2025 21:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MethodImplOptions.NoOptimization ignored for inlined function
2 participants