Skip to content

Conversation

@EgorBo
Copy link
Member

@EgorBo EgorBo commented Oct 28, 2020

A similar optimization for RyuJIT was quite profitable judging by the jit-diff: #1378

Example:

static int Test()
{
    return "hello".Length;
}

Current MONO_VERBOSE_METHOD=Test output:

IR_0000: ldstr      0
IR_0002: strlen
IR_0003: ret

New output:

IR_0000: ldc.i4.5
IR_0001: ret

@ghost
Copy link

ghost commented Oct 28, 2020

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

@BrzVlad
Copy link
Member

BrzVlad commented Oct 28, 2020

In which real world scenario does this provide us with an improvement ? Especially since, unlike ryu-jit I assume, we don't even track the string literal across variables, so it wouldn't be able to optimize the example from the linked PR, using the Validate method.

@EgorBo
Copy link
Member Author

EgorBo commented Oct 28, 2020

In which real world scenario does this provide us with an improvement ? Especially since, unlike ryu-jit I assume, we don't even track the string literal across variables, so it wouldn't be able to optimize the example from the linked PR, using the Validate method.

even without inlining like in ryujit PR (the idea of that sample was to answer the question why that optimization was not done in Roslyn) it's still useful IMO, search for ".Length in the repo (and there are more results where SOME_STR_CONST.Length is used).

@EgorBo
Copy link
Member Author

EgorBo commented Nov 1, 2020

Tests fail with

    System.Linq.Tests.TakeWhileTests.IndexTakeWhileOverflowBeyondIntMaxValueElements [SKIP]
      Condition(s) not met: "IsStressModeEnabled"
* Assertion at /__w/1/s/src/mono/mono/mini/interp/transform.c:7903, condition `sp >= stack && sp <= stack_end' not met

will fix.

@EgorBo EgorBo closed this Jan 18, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Feb 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants