RyuJIT: Extend "cns".Length optimization to support static readonly fields

[EgorBo]

#1378 introduced an optimization to fold "const str".Length into a const int. It probably makes sense to extend it to also support static readonly string fields, e.g:

Prototype: master...EgorBo:static-readonly-string-fields-opts
(it extends the existing jit-vm API getStringLiteral to accept a field reference).

jit-diff (--pmi --cctors) is not as exciting as I hoped:

(Lower is better)
Total bytes of diff: -242 (-0.00% of base)
    diff is an improvement.
Top file improvements (bytes):
        -143 : System.Diagnostics.DiagnosticSource.dasm (-0.18% of base)
         -83 : Microsoft.CodeAnalysis.dasm (-0.00% of base)
         -16 : xunit.execution.dotnet.dasm (-0.01% of base)
3 total files with Code Size differences (3 improved, 0 regressed), 264 unchanged.
Top method improvements (bytes):
        -143 (-29.12% of base) : System.Diagnostics.DiagnosticSource.dasm - Activity:GenerateRootId():String
         -71 (-18.68% of base) : Microsoft.CodeAnalysis.dasm - Reader:Read(ref,int,int):int:this
         -16 (-5.42% of base) : xunit.execution.dotnet.dasm - SerializationHelper:GetType(String,String):Type
         -12 (-26.67% of base) : Microsoft.CodeAnalysis.dasm - Reader:SetText(String):this
Top method improvements (percentages):
        -143 (-29.12% of base) : System.Diagnostics.DiagnosticSource.dasm - Activity:GenerateRootId():String
         -12 (-26.67% of base) : Microsoft.CodeAnalysis.dasm - Reader:SetText(String):this
         -71 (-18.68% of base) : Microsoft.CodeAnalysis.dasm - Reader:Read(ref,int,int):int:this
         -16 (-5.42% of base) : xunit.execution.dotnet.dasm - SerializationHelper:GetType(String,String):Type
4 total methods with Code Size differences (4 improved, 0 regressed), 258661 unchanged.
Completed analysis in 18.54s

so feel free to close it if you think it's not worth the effort.

category:cq
theme:basic-cq
skill-level:beginner
cost:small

[am11]

@EgorBo, would it happen on post-materialization phase and would it also cover tricky cases like someone is using reflection to update the value of static readonly field?

[MichalStrehovsky]

using reflection to update the value of static readonly field?

Reflection stack disallows this because there's existing codegen optimizations that rely on them being immutable. One can still break this (because nothing prevents e.g. re-running the static constructor and depending on what the static constructor does we might end up with a different value), but those are mostly academic scenarios.

[GSPP]

Maybe it should be prevented to re-run the static constructor.

[EgorBo]

@BrianBohe oops, didn't notice you self assigned this, I filed a PR that fixed a different thing but as a side effect it fixes this too

[BrianBohe]

@BrianBohe oops, didn't notice you self assigned this, I filed a PR that fixed a different thing but as a side effect it fixes this too

It's okay don't worry. I assigned it to me but I wasn't able to start it.