[wasm] Performance of GetGCStaticBase could be significantly faster

[davidwrighton]

Description

I've done an analysis of the WebAssembly code generated by the JIT for GetGCStaticBase, and I've identified a number of issues that reduce its performance.

Current state of the function ...

  (func $S_P_CoreLib_System_Runtime_CompilerServices_StaticsHelpers__GetGCStaticBase (type 58) (param i32 i32 i32) (result i32)
    (local i32 i32 i32)
    local.get 0
    i32.const 32 ;; Allocate 32bytes on stack
    i32.sub
    local.tee 0
    global.get 2
    i32.const 16141
    i32.add
    i32.store align=1 ;; Set initial Function pointer into stack frame
    block  ;; label = @1
      block  ;; label = @2
        local.get 0
        i32.const 1
        i32.store offset=4 align=1 ;; Store relative ip of 1 into stack frame
        local.get 0
        local.get 1
        i32.const 24
        i32.add
        local.tee 5
        local.get 5
        i32.const 1023
        i32.le_u
        br_if 1 (;@1;) ;; Null check on read of MethodTableAuxiliaryData pointer (pointer less than 1024)
        i32.load align=1 ;; Load MethodTableAuxiliaryData from MethodTable pointer
        i32.store offset=20 align=1 ;; Store pointer to MethodTableAuxiliaryData at offset 20.
        local.get 0
        local.get 0
        i32.load offset=20 align=1 ;; Load pointer to MethodTableAuxiliaryData from offset 20.
        i32.const -12
        i32.add
        local.tee 5
        local.get 5
        i32.const 1023 
        i32.le_u
        br_if 1 (;@1;) ;; Null check on read of gcStaticBase
        i32.load align=1 ;; read gcStaticBase from location relative to MethodTableAuxiliaryData
        i32.store offset=24 align=1 ;; Store gcStaticBase value to memory local
        local.get 0
        i32.load offset=24 align=1 ;; Load gcStaticBase value from memory local
        local.tee 3 ;; Save gcStaticBase to WASM local, and leave on stack (Not sure why this is here. We don't read the local ever)
        i32.const 1
        i32.and
        i32.const 0
        i32.ne
        br_if 0 (;@2;) ;; Check gcStaticBase against having the low bit set Fallback to GetGCStaticBaseSlow.
        local.get 0
        i32.load offset=24 align=1 ;; load gcStaticBase from memory local
        return
      end
      local.get 0
      local.get 1
      global.get 1
      i32.load offset=1026044 align=1
      local.tee 4
      local.get 4
      i32.load align=1
      call_indirect (type 58) ;; Call GetGCStaticBase slow
      return
    end
    local.get 0
    i32.const 1
    i32.store offset=4 align=1 ;; Set VirtualIP offset to 1
    local.get 0
    local.get 0
    global.get 1
    i32.load offset=968744 align=1 ;; Load address of null throw helper
    i32.store offset=28 align=1 ;; Stash PEP in memory local
    local.get 0
    i32.load offset=28 align=1 ;; load PEP from local for null throw helper for PEP argument
    local.get 0
    i32.load offset=28 align=1 ;; load PEP from local for null throw helper for function index load
    i32.load align=1           ;; load target function index
    call_indirect (type 8)
    unreachable)

In there I identified a number of issues we could fix, and measured the overall improvement to a small application which used static access very often.

1. For this function in particular, if the ByRef's it reads from are null, we could consider it to be a runtime crash, since that should never happen. So, we could remove the null checks.
2. We have a number of cases where we stored a temporary byref to the stack, but we didn't actually need to do so, since we didn't actually cross a GC point when the locals were logically live. Instead we could use WASM locals to hold the data.
3. Given that there are no memory locals after fixing issue Get core-setup building in the consolidated repo. #2, and no throwing after handling issue WIP: repo consolidation scouting kick-off - make clr build locally on Windows #1, we could get away without establishing a stack from for the stack walker to walk. We certainly don't need it at function entry. If we wanted to, we could establish the frame just before calling through GetGCStaticBaseSlow
4. Also noticed... the call to the null throw helper is using dispatch through a PEP, but for some reason it was stashing some temps into a memory based local. (The call to GetGCStaticBaseSlow was using the more efficient wasm local). Not sure what's happening there.

My hand-adjusted code looks like:

  (func $S_P_CoreLib_System_Runtime_CompilerServices_StaticsHelpers__GetGCStaticBase (type 58) (param i32 i32 i32) (result i32)
    (local i32 i32 i32)
    block  ;; label = @2
      local.get 1
      i32.const 24
      i32.add
      i32.load ;; Load MethodTableAuxiliaryData from MethodTable pointer
      i32.const -12
      i32.add
      i32.load ;; read gcStaticBase from location relative to MethodTableAuxiliaryData
      local.tee 5 ;; Store gcStaticBase pointer to WASM local
      i32.const 1
      i32.and
      i32.const 0
      i32.ne
      br_if 0 (;@1;) ;; Check gcStaticBase against having the low bit set Fallback to GetGCStaticBaseSlow.
      local.get 5 ;; Load gcStaticBase
      return
    end
    local.get 0
    local.get 1
    global.get 1
    i32.load offset=1026044 align=1
    local.tee 4
    local.get 4
    i32.load align=1
    call_indirect (type 58)
    return)

Configuration

Browser wasm R2R

Regression?

No

Data

With the above changes, a small test app which was spending about 10% of the application in this helper went from

Scenario	Performance
Initial state	28,211.44 ns/op
After removing unnecessary memory locals	27,254.35 ns/op
After also removing prolog	25,832.26 ns/op

[dotnet-policy-service]

Tagging subscribers to 'arch-wasm': @lewing, @pavelsavara
See info in area-owners.md if you want to be subscribed.

[dotnet-policy-service]

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

[davidwrighton]

A profiler indicates these changes make this particular function about 3X faster.

[AndyAyersMS]

The PEP spilling is fairly simple to fix, I'll have a PR up soon. It is a consequence of late-created calls.