Avoid using br.s when generating managed UCO lookup tables

[simonrozsival]

Contributes to #9962

The ManagedMarshalMethodsLookupGenerator generates IL of a switch statement which looks like this:

.method public hidebysig static 
    native int '<JI>GetFunctionPointer' (
        int32 methodIndex
    ) cil managed 
{
    // Method begins at RVA 0xac270
    // Header size: 1
    // Code size: 59 (0x3b)
    .maxstack 8
    
    IL_0000: ldarg methodIndex
    IL_0004: switch (IL_001b, IL_0022, IL_0029, IL_0030)
    
    IL_0019: br.s IL_0037
    
    IL_001b: ldftn void Android.App.Application::n_OnCreate_mm_wrapper(native int, native int)
    IL_0021: ret
    
    IL_0022: ldftn void Android.App.Application::n_OnLowMemory_mm_wrapper(native int, native int)
    IL_0028: ret
    
    IL_0029: ldftn void Android.App.Application::n_OnTrimMemory_I_mm_wrapper(native int, native int, int32)
    IL_002f: ret
    
    IL_0030: ldftn void Android.App.Application::n_OnConfigurationChanged_Landroid_content_res_Configuration__mm_wrapper(native int, native int, native int)
    IL_0036: ret
    
    IL_0037: nop
    IL_0038: ldc.i4.m1
    IL_0039: conv.i
    IL_003a: ret
} // end of method Application::'<JI>GetFunctionPointer'

If the switch instruction doesn't jump to any of the target offsets, it continues on the next instruction. This instruction used to be a br.s which would jump over all the individual cases to the default case. When the switch got too large (19 cases), the br.s became insufficient (the offset for br.s is a signed 8-bit number).

The code can be simplified by removing the branching operation completely and simply return from the default case. The newly generated code will look like this:

.method public hidebysig static 
    native int '<JI>GetFunctionPointer' (
        int32 methodIndex
    ) cil managed 
{
    // Method begins at RVA 0xac1a8
    // Header size: 1
    // Code size: 56 (0x38)
    .maxstack 8
    
    IL_0000: ldarg methodIndex
    IL_0004: switch (IL_001c, IL_0023, IL_002a, IL_0031)
    
    IL_0019: ldc.i4.m1
    IL_001a: conv.i
    IL_001b: ret
    
    IL_001c: ldftn void Android.App.Application::n_OnCreate_mm_wrapper(native int, native int)
    IL_0022: ret
    
    IL_0023: ldftn void Android.App.Application::n_OnLowMemory_mm_wrapper(native int, native int)
    IL_0029: ret
    
    IL_002a: ldftn void Android.App.Application::n_OnTrimMemory_I_mm_wrapper(native int, native int, int32)
    IL_0030: ret
    
    IL_0031: ldftn void Android.App.Application::n_OnConfigurationChanged_Landroid_content_res_Configuration__mm_wrapper(native int, native int, native int)
    IL_0037: ret
} // end of method Application::'<JI>GetFunctionPointer'

[ivanpovazan]

I know it is not related to this PR, but should we also cleanup - remove these lines:

android/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets

Lines 349 to 351 in 4a8dd1e

	<!-- NOTE: temporarily disable for CoreCLR for now, until we have an implementation that works
	with the new runtime -->
	<AndroidEnableMarshalMethods Condition=" '$(AndroidEnableMarshalMethods)' == '' and '$(_AndroidRuntime)' == 'CoreCLR' ">False</AndroidEnableMarshalMethods>

As they are not relevant anymore?

Line:

android/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets

Line 345 in 4a8dd1e

<AndroidEnableMarshalMethods Condition=" '$(AndroidEnableMarshalMethods)' == '' and '$(_AndroidRuntime)' != 'NativeAOT' ">True</AndroidEnableMarshalMethods>

already sets AndroidEnableMarshalMethods=true for non-NativeAOT runtimes

[simonrozsival]

@ivanpovazan that should definitely be changed. I think we should do it in a separate PR though.