Skip to content

RyuJIT: Fold "cns_str"[cns_index] to ushort constant#37226

Merged
sandreenko merged 1 commit intodotnet:masterfrom
EgorBo:fold-cns-str-index
Jul 13, 2020
Merged

RyuJIT: Fold "cns_str"[cns_index] to ushort constant#37226
sandreenko merged 1 commit intodotnet:masterfrom
EgorBo:fold-cns-str-index

Conversation

@EgorBo
Copy link
Member

@EgorBo EgorBo commented May 31, 2020

A small improvement to fold things like "hello"[1] to 'e' in RyuJIT

jit-diff (-f -pmi):

Total bytes of diff: -209 (-0.00% of base)
    diff is an improvement.

Top file improvements (bytes):
        -209 : System.Private.CoreLib.dasm (-0.00% of base)

1 total files with Code Size differences (1 improved, 0 regressed), 265 unchanged.

Top method improvements (bytes):
         -81 (-1.41% of base) : System.Private.CoreLib.dasm - System.DateTimeFormat:FormatCustomized(System.DateTime,System.ReadOnlySpan`1[Char],System.Globalization.DateTimeFormatInfo,System.TimeSpan,System.Text.StringBuilder):System.Text.StringBuilder
         -39 (-19.31% of base) : System.Private.CoreLib.dasm - System.Globalization.DateTimeFormatInfo:IsAllowedJapaneseTokenFollowedByNonSpaceLetter(System.String,ushort):bool:this
         -33 (-1.72% of base) : System.Private.CoreLib.dasm - System.Globalization.DateTimeFormatInfo:Tokenize(int,byref,byref,byref):bool:this
         -29 (-0.56% of base) : System.Private.CoreLib.dasm - System.DateTimeParse:ParseByFormat(byref,byref,byref,System.Globalization.DateTimeFormatInfo,byref):bool
         -27 (-15.43% of base) : System.Private.CoreLib.dasm - System.DateTimeParse:ParseJapaneseEraStart(byref,System.Globalization.DateTimeFormatInfo):bool

Top method improvements (percentages):
         -39 (-19.31% of base) : System.Private.CoreLib.dasm - System.Globalization.DateTimeFormatInfo:IsAllowedJapaneseTokenFollowedByNonSpaceLetter(System.String,ushort):bool:this
         -27 (-15.43% of base) : System.Private.CoreLib.dasm - System.DateTimeParse:ParseJapaneseEraStart(byref,System.Globalization.DateTimeFormatInfo):bool
         -33 (-1.72% of base) : System.Private.CoreLib.dasm - System.Globalization.DateTimeFormatInfo:Tokenize(int,byref,byref,byref):bool:this
         -81 (-1.41% of base) : System.Private.CoreLib.dasm - System.DateTimeFormat:FormatCustomized(System.DateTime,System.ReadOnlySpan`1[Char],System.Globalization.DateTimeFormatInfo,System.TimeSpan,System.Text.StringBuilder):System.Text.StringBuilder
         -29 (-0.56% of base) : System.Private.CoreLib.dasm - System.DateTimeParse:ParseByFormat(byref,byref,byref,System.Globalization.DateTimeFormatInfo,byref):bool

5 total methods with Code Size differences (5 improved, 0 regressed), 312722 unchanged.

/cc @sandreenko

@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 May 31, 2020
@stephentoub
Copy link
Member

stephentoub commented May 31, 2020

What's an example of where we do this pattern? Does this arise mainly due to inlining?

@EgorBo
Copy link
Member Author

EgorBo commented May 31, 2020 •
edited
Loading

@stephentoub in one of the affected methods it does JapaneseEraStart[0] where JapaneseEraStart is a constant string. I expected a bit bigger diff due to inlining (it's not really possible to estimate impact until you implement an optimization) but maybe it's possible to extend it for spans, e.g.:

void Format(ReadOnlySpan<char> format, ...) {  if (format[0] == 'G') { ... } }

// caller:
Format("G",...);

@stephentoub
Copy link
Member

stephentoub commented Jun 1, 2020

Thanks

@BruceForstall
Copy link
Contributor

BruceForstall commented Jun 1, 2020

@sandreenko @AndyAyersMS

sandreenko
sandreenko approved these changes Jul 13, 2020
View reviewed changes
Copy link
Contributor

@sandreenko sandreenko left a comment

Choose a reason for hiding this comment

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

LGTM, thank you

@sandreenko sandreenko merged commit b182639 into dotnet:master Jul 13, 2020
@EgorBo EgorBo mentioned this pull request Dec 4, 2020
Closed
@ghost ghost locked as resolved and limited conversation to collaborators Dec 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@sandreenko sandreenko sandreenko approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

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.

5 participants

@EgorBo @stephentoub @BruceForstall @sandreenko @Dotnet-GitSync-Bot