Have mono handle the vector as APIs that grow or shrink the vector type

[tannergooding]

No description provided.

[dotnet-policy-service]

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

[kg]

There might be a problem here - if the v3 is in a stack local, it's 16 bytes wide and you might need to zero [3]. I'm not sure whether res can be a non-stack address though...

[tannergooding]

I had code earlier that tried to always handle it as 16-bytes, but it didn't help.

Notably, I wouldn't expect the need to explicitly zero that space anyways as it would be considered padding space and should be ignored when loaded. Otherwise it would risk corrupting other operations, like Sum.

[kg]

I don't know a lot about the minijit side of things, but I can try to look into the interp stuff next week.

[kg]

I'll try to reproduce the wasm interp failures locally and see if I can figure them out. The minijit one is way beyond my knowledge, but if you're stuck and no one else can help, you can tag me in for that too.

[tannergooding]

I'll try to reproduce the wasm interp failures locally and see if I can figure them out. The minijit one is way beyond my knowledge, but if you're stuck and no one else can help, you can tag me in for that too.

Thanks much!

For the minijit one I'm pretty sure I understand the basic problem but I'd like to at least get the larger interp failures addressed first 😅

[kg]

I haven't been able to run this locally because wasm builds are still broken for me by the libz stuff, which also seems to have broken codespaces. I'll see if I can get a successful build on my windows device and troubleshoot there.

[kg]

This one:

[01:25:45] warn: [MONO] Encountered infinite recursion while looking up resource 'InvalidProgram_Default' in System.Private.CoreLib. Verify the installation of .NET is complete and does not need repairing, and that the state of the process has not become corrupted.

Appears to be an InvalidProgramException in transform.c's handling of CEE_RET, which looks like this:

			if (td->sp > td->stack) {
				mono_error_set_generic_error (error, "System", "InvalidProgramException", "stack overflow in CEE_RET");
				goto exit;
			}

This suggests that something in this PR is unbalancing the interpreter stack.

EDIT: Specifically in Ascii.HasMatch

[kg]

It looks like somehow the ceq operation after the callvirt is being eaten, which leaves garbage on the stack and causes the interp to reject the code as malformed. Compare the source IL and the trace afterward:

  Method System.Text.Ascii:HasMatch<System.Runtime.Intrinsics.Vector128`1<byte>> (System.Runtime.Intrinsics.Vector128`1<byte>), optimized 0, original code:
  IL_0000: ldarg.0   
  IL_0001: ldc.i4    128
  IL_0006: constrained.0x1b000024
  IL_000c: call      0x0a000e3a
  IL_0011: constrained.0x1b000024
  IL_0017: call      0x0a000099
  IL_001c: stloc.0   
  IL_001d: ldloca.s  0
  IL_001f: constrained.0x1b000024
  IL_0025: call      0x0a000e3b
  IL_002a: constrained.0x1b000024
  IL_0030: callvirt  0x0a0006d9
  IL_0035: ldc.i4.0  
  IL_0036: ceq       
  IL_0038: ret       
  

  IL_0000 ldarg.0   , sp 0,                
  IL_0001 ldc.i4    , sp 1, VT Vector128`1 
  IL_0006 prefix1   , sp 2, I4             
  IL_000c call      , sp 2, I4             
  IL_0011 prefix1   , sp 2, VT Vector128`1 
  IL_0017 call      , sp 2, VT Vector128`1 
  IL_001c stloc.0   , sp 1, VT Vector128`1 
  IL_001d ldloca.s  , sp 0,                
  IL_001f prefix1   , sp 1, MP             
  IL_0025 call      , sp 1, MP             
  IL_002a prefix1   , sp 2, VT Vector128`1 
  IL_0030 callvirt  , sp 2, VT Vector128`1 
  Vector128<T>.Equals produced intrinsic 16
  IL_0035 ldc.i4.0  , sp 2, I4             
  IL_0038 ret       , sp 2, I4             
  Ascii.HasMatch: stack overflow in CEE_RET (td->sp 0x55fc188f3f88 > td->stack 0x55fc188f3f70)
  Process terminated.

EDIT: It looks like maybe this is an optimization baked into the decoder for LDC_I4_0:

[kg]

OK, the missing ceq is a red herring, disabling the optimization responsible for that doesn't fix it.

EDIT: I think emit_common_simd_epilogue is missing support for csignature->hasthis.

[kg]

Changing emit_common_simd_epilogue like so fixes the crash:

emit_common_simd_epilogue (TransformData *td, MonoClass *vector_klass, MonoMethodSignature *csignature, int vector_size, gboolean allow_void)
{
+	if (csignature->hasthis)
+		td->sp--;
	td->sp -= csignature->param_count;

I'm not sure that's quite right though.

[tannergooding]

Thanks so much for the help here! I believe the new commit should correctly handle instance methods more generally speaking now.

I'll still want to fix the jiterpreter to handle these new cases as well and ensure the MonoJIT/AOT path is passing, but I think I should be unblocked now!

[tannergooding]

Now that the same size bitcasts are well handled and showing the expected perf improvements, I plan on cleaning this up next week, but I expect it won't land for .NET 9 before the Preview 7 snap

[tannergooding]

Going to close this one. There's some other changes that are needed for mono-wasm before it can be done there (namely support for WithElement), so I'll try to get those up independently first.