We have two places in CoreLib that callUnsafe.InitBlockUnaligned. These places are effectively going to callSpanHelpers.Fill now - we can fix them up to do that directly.

We have two places in CoreLib that callUnsafe.InitBlockUnaligned. These places are effectively going to callSpanHelpers.Fill now - we can fix them up to do that directly.

The problem thatSpanHelpers.Fill is not optimized by jit for constant length. It was never needed, because e.g.Span.Clear invokedUnsafe.InitBlockUnaligned

No longer happens if we change it to callSpanHelpers.Fill directly. I'll look in a separate PR to unroll Fill as well (should be a simple fix).

@jkotas we don't use memset/memcpy helpers in JIT for x86 and always userep stosb/movb - wonder if we should switch those to helpers or not? I presumerep might also lead to gc starvation.

I presume rep might also lead to gc starvation.

rep ... should be emitted as fully interruptible code to avoid gc starvation. It is same as a loop without a call.

Btw, I found & fixed an existing bug - Memmove used to not throw an NRE if both src and dest are null + length is not zero e.g.:

Buffer.MemoryCopy(null,null,10,10);

this doesn't fail (likely can be reproduced in other APIs which use Memmove under the hood)

Memmove used to not throw an NRE if both src and dest are null + length is not zero e.g.

This is not a bug that needs fixing. Passing invalid buffer to Memmove is UB.

Memmove used to not throw an NRE if both src and dest are null + length is not zero e.g.

This is not a bug that needs fixing. Passing invalid buffer to Memmove is UB.

if I don't fix it, then:

voidTest(MyStruct*a,MyStruct*b)=>*a=*b;// or its ref version

won't fail on both being null (or will depend on opts level)

Ok, I agree that it makes sense to fix this for internal JIT use of memory copy.

Done. I wrote somebenchmarks
whereCore_Root_PR_tailcall is this PR + a conservativefix for taillcalls (don't see big improvements from this). The fix is conservative because it disables unrollings for constant sizes.

NOTE: PGO doesn't unroll these unknown-size memops because the opt I landed for this currently require R2R=0 and I didn't disable it.

Overall looks good to me, IMO. It's possible that in some cases helper calls are always indirect because they're promoted to Tier1 together with the benchmarks, for cast helpers we didthis trick, but I am not sure we want to add the whole SpanHelpers here as well.

CopyBlock4000/InitBlock4000 likely struggle from going to native memset/memmove anyway from the managed helpers
It is also possible that these numbers vary a bit from run to run due to GC alignment (although, I tried to use[MemoryRandomization] I am not sure it helps).

a conservativefix for taillcalls

Where would you expect it to help with the current state of the PR? The wrappers for JIT helpers are not there anymore, so the JIT helpers are not affected by this.

for cast helpers we didthis trick,

This sounds like a general problem with ordering of tiered compilation. For direct calls, it would be preferable to compile the bottom layer methods faster than upper layer methods. I do not think we have anything in place to help with that (correct me if I am wrong). If there are a few more bottom layer types that we care about in particular, I do not see a problem with adding them to this workaround.

Is the SpanHelpers byte overload more efficient for sizeof(T) == 1 or there is no difference?

No difference for now, but I wanted to improve the byte version so there will be

Is this left for a follow up?

Where would you expect it to help with the current state of the PR?

For ordinary managed calls for Clear/Memmove inside void methods, example:

voidTest(Span<byte>span)=>span.Clear();

^ the codegen is expected to perform a tail call toSpanHelpers.ClearWithoutReferences (it doesn't happen with this PR yet)

If there are a few more bottom layer types that we care about in particular, I do not see a problem with adding them to this workaround.

Overall I agree, it's just that I'd rather wantSpanHelpers to be promoted with a delay to avoid setting in stone weights too early (one day we'll implement the context-sensitive PGO to solve this problem differently). Also, all R2R'd methods can stay in R2R form longer. This timing issue can only manifest in simple microbenchmarks I presume (I still not sure the perf difference between indirect and direct calls are noticeable)

Is this left for a follow up?

Yep. We probably can ask our ARM64/Intel contributors for help here, although, I am not sure non-zero Memset is a popular operation (very few hits accross SPMI).