Skip to content

JIT: optimize non-constant stackalloc to constant #97895

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.

Sign up for GitHub

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

Jump to bottom
Closed
EgorBo wants to merge 2 commits into from
Closed

JIT: optimize non-constant stackalloc to constant #97895

EgorBo wants to merge 2 commits into from

Conversation

@EgorBo
Copy link
Member

@EgorBo EgorBo commented Feb 2, 2024 •
edited
Loading

This PR optimizes

span = x <= 64 ? stackalloc byte[n] : ...

to

span = x <= 64 ? stackalloc byte[64] : ...

because the latter is faster to zero (single instruction). Although, this consumes more stack, so the threshold is low.

Also, I need the generic optAssertionProp_GetInt32Range function in future opts I wanted to try.

Example:

static void Test(int n)
{
    Span<byte> span = (uint)n <= 64 ? stackalloc byte[n] : new byte[n];
    Consume(span);
}

[MethodImpl(MethodImplOptions.NoInlining)]
static void Consume(Span<byte> span){}

Codegen diff for Test(): https://www.diffchecker.com/s6pIRCSa/

@ghost ghost assigned EgorBo Feb 2, 2024
@ghost ghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Feb 2, 2024
@ghost
Copy link

ghost commented Feb 2, 2024

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

Issue Details

This PR optimizes

span = x < 64 ? stackalloc byte[n] : ...

to

span = x < 64 ? stackalloc byte[64] : ...

because the latter is faster to zero (single instruction). Although, this consumes more stack so the threshold is low.

Also, I need the generic optAssertionProp_GetInt32Range function in some other opts.

Author: EgorBo
Assignees: EgorBo
Labels:

area-CodeGen-coreclr
Milestone: -

@stephentoub
Copy link
Member

stephentoub commented Feb 2, 2024

span = x <= 64 ? stackalloc byte[n] : ...
to
span = x <= 64 ? stackalloc byte[64] : ...

Does it slice it as well? Otherwise, that's a visible change that's almost guaranteed to break code that depended on span.Length being n rather than 64.

@EgorBo
Copy link
Member Author

EgorBo commented Feb 2, 2024 •
edited
Loading

span = x <= 64 ? stackalloc byte[n] : ...
to
span = x <= 64 ? stackalloc byte[64] : ...

Does it slice it as well? Otherwise, that's a visible change that's almost guaranteed to break code that depended on span.Length being n rather than 64.

Right, it doesn't change that behaviour, my example is not entirely correct from C# point of view, but it gives the idea of what JIT does for it under the hood.

@EgorBo
Copy link
Member Author

EgorBo commented Feb 2, 2024 •
edited
Loading

Also, this PR closes #81160 (although, I didn't plan it)

@jakobbotsch
Copy link
Member

jakobbotsch commented Feb 2, 2024

What is the escape hatch for a user that truly wants to allocate as little stack as possible? In general I think we should be careful around changing stack consumption for patterns like this unless we have good evidence of benefits to motivate it (stack consumption vs execution speed is, as you say, a tradeoff)...

@EgorBo
Copy link
Member Author

EgorBo commented Feb 2, 2024 •
edited
Loading

What is the escape hatch for a user that truly wants to allocate as little stack as possible? In general I think we should be careful around changing stack consumption for patterns like this unless we have good evidence of benefits to motivate it (stack consumption vs execution speed is, as you say, a tradeoff)...

This is just a draft to see the diffs. I wrote this function for a different opt, just noticed that it's a few lines of code to apply it for LCLHEAP. I don't have an answer on what threshold is safe to use to potentially increase stack usage for better perf.

@build-analysis build-analysis bot mentioned this pull request Feb 2, 2024
Closed
@EgorBo
Copy link
Member Author

EgorBo commented Feb 2, 2024

Closing since I am not feeling confident enough to increase stack consumption in such functions.
I'll move my other changes to a separate PR and leave this as a history record

@EgorBo EgorBo closed this Feb 2, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Mar 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

Copilot code review Copilot Awaiting requested review from Copilot Copilot will automatically review once the pull request is marked ready for review

Assignees

@EgorBo EgorBo

Labels

area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@EgorBo @stephentoub @jakobbotsch