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

JIT: Fold constant access to [ReadOnly]Span<T> especially for char and byte #64823

Closed
EgorBo opened this issue Feb 4, 2022 · 2 comments · Fixed by #78783
Closed

JIT: Fold constant access to [ReadOnly]Span<T> especially for char and byte #64823

EgorBo opened this issue Feb 4, 2022 · 2 comments · Fixed by #78783
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI help wanted [up-for-grabs] Good issue for external contributors
Milestone

Comments

@EgorBo
Copy link
Member

EgorBo commented Feb 4, 2022

In order to be able to declare unrolling rules in pure C# the way it was proposed in #64821 for strings we need to be able to fold this in JIT:

T SecondChar<T>(ReadOnlySpan<T> span) => span[1];
T GetLength<T>(ReadOnlySpan<T> span) => span.Length;

char Test1() => SecondChar<char>("hello"); // => 'e'
int  Test2() => GetLength<char>("hello"); // => 5

Both Test1 and Test2 has to be folded to constants.
Ideally we should be able to also handle spans of bytes (utf8)

Also, fold constant accesses to RVA data (from #64612). Example:

static ReadOnlySpan<byte> data => new byte[] { 1, 2, 3, 4, 5 };

byte Test() => data[3];

Current codegen:

; Method Prog:Test():ubyte:this
       48B8A32BAD36FD010000 mov      rax, 0x1FD36AD2BA3
       0FB600               movzx    rax, byte  ptr [rax]
       C3                   ret      
; Total bytes of code: 14

Expected codegen:

; Method Prog:Test():ubyte:this
       B804000000           mov      eax, 4
       C3                   ret      
; Total bytes of code: 6

NOTES:

  • JIT is already able to do it for string literals e.g. "hello"[2] (see fgMorphArrayIndex)
  • C# should soon get a support for ROS<any primitive> = RVA, not just byte/bool as it is currently
  • We probably can't do it for RVAs in C++/CLI/CX which are mutable (check for mixed-mode)
  • This task most likely needs a new JIT-EE API or maybe the existing getArrayInitializationData is enough
@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Feb 4, 2022
@EgorBo EgorBo added area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI help wanted [up-for-grabs] Good issue for external contributors labels Feb 4, 2022
@ghost
Copy link

ghost commented Feb 4, 2022

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

Issue Details

In order to be able to declare unrolling rules in pure C# the way it was proposed in #64821 for strings we need to be able to fold this in JIT:

T Bar<T>(ReadOnlySpan<T> span) => span[0];

char Foo1() => Bar<char>("hello");
byte Foo2() => Bar<byte>(new byte[] { 1,2,3 });

Both Foo1 and Foo2 has to be folded to constants.

Author: EgorBo
Assignees: -
Labels:

up-for-grabs, area-CodeGen-coreclr, untriaged

Milestone: -

@EgorBo EgorBo added this to the Future milestone Feb 4, 2022
@EgorBo EgorBo removed the untriaged New issue has not been triaged by the area owner label Feb 4, 2022
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Nov 23, 2022
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Nov 29, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Dec 29, 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 help wanted [up-for-grabs] Good issue for external contributors
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant