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]

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.

[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]

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.

[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]

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.

[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]

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.

[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]

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.

[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]

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.

[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]

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.

[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]

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.