Skip to content

Commit

Permalink
[Mono] Fix all the issues related to enabling WasmStripILAfterAOT (do…
Browse files Browse the repository at this point in the history
…tnet#90436)

* Enable IL trim for WASM by default

* Make ILStrip available for local build

* Make all calling another method go through the logic to see if it could call an AOT'ed version of it before trying to interp compile it

* Add back accidentally removed line of code

* Update test to accommodate IL trim with WASM AOT

* Move jit_call_can_be_supported to mini-runtime, so it doesn't depends on the value of mono_use_interpreter

* Update var name

* Attempt to fix rebuild test failures

* Attempt to fix the file open issue with unicode on windows

* Attempt to fix unicode issue #2

* Enable g_fopen to have the capability of handling opening files with unicode name on all platforms

* Add comment

* Update comment

* Fix file indentation format

* Check if string contains non-ascii char

* Remove unused callback

* Remove redundant comment

* Update method name

* Fixed some method not found issues and remove the optimization for g_fopen

* Fix tailcall

* Disable tailcall optimization when calling a trimmed method

* Free method header

* Fix windows build error

* Free method header at the correct locatioin

* Fix the condition of skipping tailcall

* Fix test failure

* Move JIT/AOT call invoke away from MINT_CALL, as it is not needed there.

* Fix virtual tail call

* Address review feedback

* Put the trimmed assemblies in a new folder, output an updated list of assemblies and update _WasmAssembliesInternal with the new list

* Put trimmed assemblies in IntermediateOutputPath

* Remove TrimmedAssemblies

* Create trimmed assembly folder before the parallel run

* Try to fix the issue with missing item

* Fix parallelism issue

* Only start the trim when the assembly is newer than the output

* Add assembly item to the list, when

* Add some logging

* Fixed runtimeconfig.json file path issue and disabed failed tests

* Update parameter name

* Fix wasi build

* Use the correct parameter

* Fix runtime test failure

* WasmAppBuilder: runtimeconfig.json path can be null

* cleanup

* ILStrip: fix typo in id name

* Cleanup

* ILStrip: ensure output assemblies are in the same order as the input. This is required for incremental builds.

* more cleanup

* Re-enable disabled tests

* Change the default value for WasmStripILAfterAOT to false

* Fix the issue with changing the value of WasmStripILAfterAOT between incremental builds

* Move the location of file deleting

* Use WasmAssembliesFinal or ResolvedFileToPublish during publish in WasmSDK

* Copy metadata in AOT compiler and when creating WasmAssembliesToBundle from ResolvedFileToPublish

* Add _WasmSatelliteAssemblies to WasmAssembliesFinal

* Add a wasm template test

* Include all non-dll ResolvedFileToPublish for ComputeWasmPublishAssets

* Add a blazor template test

* Address review feedback

* Update src/mono/wasm/Wasm.Build.Tests/Templates/WasmTemplateTests.cs

Co-authored-by: Ankit Jain <[email protected]>

* Update parameter name

* Update usage of parameter

* Revert EmccCompile change

* MonoAOTCompiler: revert rebuilding when WasmStripILAfterAOT value changes

* Compress -> GZipCompress, and cleanup

* rework the tests

- to support webcil case
- run blazor app
- add cases for the default setting, and the opposite
- cleanup

* ILStrip.cs: Emit a message about stripping to make it obvious to the user

* WasmApp.targets: update comment

* Change default value to false and update test

---------

Co-authored-by: Ankit Jain <[email protected]>
Co-authored-by: Marek Fišera <[email protected]>
  • Loading branch information
3 people authored Nov 13, 2023
1 parent 0e6e35f commit 085ddb7
Show file tree
Hide file tree
Showing 23 changed files with 475 additions and 184 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -55,27 +55,44 @@ public static void TestMethodBody()
{
MethodBase mbase = typeof(MethodBaseTests).GetMethod("MyOtherMethod", BindingFlags.Static | BindingFlags.Public);
MethodBody mb = mbase.GetMethodBody();
var codeSize = mb.GetILAsByteArray().Length;
Assert.True(mb.InitLocals); // local variables are initialized
#if DEBUG
Assert.Equal(2, mb.MaxStackSize);
Assert.Equal(3, mb.LocalVariables.Count);

foreach (LocalVariableInfo lvi in mb.LocalVariables)
if (codeSize == 0)
{
if (lvi.LocalIndex == 0) { Assert.Equal(typeof(int), lvi.LocalType); }
if (lvi.LocalIndex == 1) { Assert.Equal(typeof(string), lvi.LocalType); }
if (lvi.LocalIndex == 2) { Assert.Equal(typeof(bool), lvi.LocalType); }
}
// This condition is needed for running this test under WASM AOT mode.
// Because IL trim is enabled be default for WASM apps whenever AOT is enabled.
// And the method body of "MyOtherMethod" will be trimmed.
#if DEBUG
Assert.Equal(2, mb.MaxStackSize);
#else
Assert.Equal(1, mb.MaxStackSize);
Assert.Equal(2, mb.LocalVariables.Count);

foreach (LocalVariableInfo lvi in mb.LocalVariables)
{
if (lvi.LocalIndex == 0) { Assert.Equal(typeof(int), lvi.LocalType); }
if (lvi.LocalIndex == 1) { Assert.Equal(typeof(string), lvi.LocalType); }
Assert.Equal(1, mb.MaxStackSize);
#endif
Assert.Equal(0, mb.LocalVariables.Count);
}
else
{
#if DEBUG
Assert.Equal(2, mb.MaxStackSize);
Assert.Equal(3, mb.LocalVariables.Count);

foreach (LocalVariableInfo lvi in mb.LocalVariables)
{
if (lvi.LocalIndex == 0) { Assert.Equal(typeof(int), lvi.LocalType); }
if (lvi.LocalIndex == 1) { Assert.Equal(typeof(string), lvi.LocalType); }
if (lvi.LocalIndex == 2) { Assert.Equal(typeof(bool), lvi.LocalType); }
}
#else
Assert.Equal(1, mb.MaxStackSize);
Assert.Equal(2, mb.LocalVariables.Count);

foreach (LocalVariableInfo lvi in mb.LocalVariables)
{
if (lvi.LocalIndex == 0) { Assert.Equal(typeof(int), lvi.LocalType); }
if (lvi.LocalIndex == 1) { Assert.Equal(typeof(string), lvi.LocalType); }
}
#endif
}
}

private static int MyAnotherMethod(int x)
Expand Down
25 changes: 25 additions & 0 deletions src/mono/mono/eglib/gfile.c
Original file line number Diff line number Diff line change
Expand Up @@ -104,3 +104,28 @@ g_file_error_from_errno (gint err_no)
return G_FILE_ERROR_FAILED;
}
}

FILE *
g_fopen (const char *path, const char *mode)
{
FILE *fp;

if (!path)
return NULL;

#ifndef HOST_WIN32
fp = fopen (path, mode);
#else
gunichar2 *wPath = g_utf8_to_utf16 (path, -1, 0, 0, 0);
gunichar2 *wMode = g_utf8_to_utf16 (mode, -1, 0, 0, 0);

if (!wPath || !wMode)
return NULL;

fp = _wfopen ((wchar_t *) wPath, (wchar_t *) wMode);
g_free (wPath);
g_free (wMode);
#endif

return fp;
}
1 change: 1 addition & 0 deletions src/mono/mono/eglib/glib.h
Original file line number Diff line number Diff line change
Expand Up @@ -962,6 +962,7 @@ typedef enum {

G_ENUM_FUNCTIONS (GFileTest)

FILE * g_fopen (const char *path, const char *mode);
gboolean g_file_get_contents (const gchar *filename, gchar **contents, gsize *length, GError **gerror);
GFileError g_file_error_from_errno (gint err_no);
gint g_file_open_tmp (const gchar *tmpl, gchar **name_used, GError **gerror);
Expand Down
13 changes: 9 additions & 4 deletions src/mono/mono/mini/aot-compiler.c
Original file line number Diff line number Diff line change
Expand Up @@ -9885,10 +9885,15 @@ compile_method (MonoAotCompile *acfg, MonoMethod *method)
mono_atomic_inc_i32 (&acfg->stats.ccount);

if (acfg->aot_opts.trimming_eligible_methods_outfile && acfg->trimming_eligible_methods_outfile != NULL) {
if (!mono_method_is_generic_impl (method) && method->token != 0 && !cfg->deopt && !cfg->interp_entry_only && mini_get_interp_callbacks ()->jit_call_can_be_supported (method, mono_method_signature_internal (method), acfg->aot_opts.llvm_only)) {
// The call back to jit_call_can_be_supported is necessary for WASM, because it would still interprete some methods sometimes even though they were already AOT'ed.
if (!mono_method_is_generic_impl (method) && method->token != 0 && !cfg->deopt && !cfg->interp_entry_only) {
// The call to mono_jit_call_can_be_supported_by_interp is necessary for WASM, because it would still interprete some methods sometimes even though they were already AOT'ed.
// When that happens, interpreter needs to have the capability to call the AOT'ed version of that method, since the method body has already been trimmed.
fprintf (acfg->trimming_eligible_methods_outfile, "%x\n", method->token);
gboolean skip_trim = FALSE;
if (acfg->aot_opts.interp) {
skip_trim = (!mono_jit_call_can_be_supported_by_interp (method, mono_method_signature_internal (method), acfg->aot_opts.llvm_only) || (method->iflags & METHOD_IMPL_ATTRIBUTE_SYNCHRONIZED));
}
if (!skip_trim)
fprintf (acfg->trimming_eligible_methods_outfile, "%x\n", method->token);
}
}
}
Expand Down Expand Up @@ -14917,7 +14922,7 @@ aot_assembly (MonoAssembly *ass, guint32 jit_opts, MonoAotOptions *aot_options)
}

if (acfg->aot_opts.trimming_eligible_methods_outfile && acfg->dedup_phase != DEDUP_COLLECT) {
acfg->trimming_eligible_methods_outfile = fopen (acfg->aot_opts.trimming_eligible_methods_outfile, "w+");
acfg->trimming_eligible_methods_outfile = g_fopen (acfg->aot_opts.trimming_eligible_methods_outfile, "w");
if (!acfg->trimming_eligible_methods_outfile)
aot_printerrf (acfg, "Unable to open trimming-eligible-methods-outfile specified file %s\n", acfg->aot_opts.trimming_eligible_methods_outfile);
else {
Expand Down
1 change: 0 additions & 1 deletion src/mono/mono/mini/ee.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ typedef gpointer MonoInterpFrameHandle;
MONO_EE_CALLBACK (void, entry_llvmonly, (gpointer res, gpointer *args, gpointer imethod)) \
MONO_EE_CALLBACK (gpointer, get_interp_method, (MonoMethod *method)) \
MONO_EE_CALLBACK (MonoJitInfo*, compile_interp_method, (MonoMethod *method, MonoError *error)) \
MONO_EE_CALLBACK (gboolean, jit_call_can_be_supported, (MonoMethod *method, MonoMethodSignature *sig, gboolean is_llvm_only)) \

typedef struct _MonoEECallbacks {

Expand Down
6 changes: 0 additions & 6 deletions src/mono/mono/mini/interp-stubs.c
Original file line number Diff line number Diff line change
Expand Up @@ -252,12 +252,6 @@ stub_compile_interp_method (MonoMethod *method, MonoError *error)
return NULL;
}

static gboolean
stub_jit_call_can_be_supported (MonoMethod *method, MonoMethodSignature *sig, gboolean is_llvm_only)
{
return TRUE;
}

#undef MONO_EE_CALLBACK
#define MONO_EE_CALLBACK(ret, name, sig) stub_ ## name,

Expand Down
3 changes: 0 additions & 3 deletions src/mono/mono/mini/interp/interp-internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -320,9 +320,6 @@ mono_mint_type (MonoType *type);
int
mono_interp_type_size (MonoType *type, int mt, int *align_p);

gboolean
interp_jit_call_can_be_supported (MonoMethod *method, MonoMethodSignature *sig, gboolean is_llvm_only);

#if HOST_BROWSER

gboolean
Expand Down
Loading

0 comments on commit 085ddb7

Please sign in to comment.