Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[mono] Extend mono AOT compiler to ingest .mibc profiles #70194

Merged
merged 6 commits into from
Jun 6, 2022
Merged

[mono] Extend mono AOT compiler to ingest .mibc profiles #70194

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7038cbe
[mono] Add non_executable field to MonoImage to properly load MethodR…
mdh1418 Jun 3, 2022
46d2e03
[mono][aot] Extend mono-aot-compiler to handle .mibc profile
mdh1418 Jun 3, 2022
406ced3
[tasks] Extend MonoAOTCompiler task to handle .mibc profiles
mdh1418 Jun 3, 2022
7428239
Address feedback
mdh1418 Jun 3, 2022
b1fb61e
Add method descriptions
mdh1418 Jun 4, 2022
4774439
Address more feedback
mdh1418 Jun 6, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 17 additions & 18 deletions src/mono/mono/metadata/assembly.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -969,24 +969,21 @@ mono_assembly_load_reference (MonoImage *image, int index)
goto commit_reference;
}

if (image->assembly) {
if (mono_trace_is_traced (G_LOG_LEVEL_INFO, MONO_TRACE_ASSEMBLY)) {
char *aname_str = mono_stringify_assembly_name (&aname);
mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_ASSEMBLY, "Loading reference %d of %s (%s), looking for %s",
index, image->name, mono_alc_is_default (mono_image_get_alc (image)) ? "default ALC" : "custom ALC" ,
aname_str);
g_free (aname_str);
}

MonoAssemblyByNameRequest req;
mono_assembly_request_prepare_byname (&req, mono_image_get_alc (image));
req.requesting_assembly = image->assembly;
//req.no_postload_search = TRUE; // FIXME: should this be set?
reference = mono_assembly_request_byname (&aname, &req, NULL);
} else {
g_assertf (image->assembly, "While loading reference %d MonoImage %s doesn't have a MonoAssembly", index, image->name);
g_assertf (image->assembly || image->not_executable, "While loading reference %d, executable MonoImage %s doesn't have a MonoAssembly", index, image->name);
if (mono_trace_is_traced (G_LOG_LEVEL_INFO, MONO_TRACE_ASSEMBLY)) {
char *aname_str = mono_stringify_assembly_name (&aname);
mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_ASSEMBLY, "Loading reference %d of %s (%s), looking for %s",
index, image->name, mono_alc_is_default (mono_image_get_alc (image)) ? "default ALC" : "custom ALC" ,
aname_str);
g_free (aname_str);
}

MonoAssemblyByNameRequest req;
mono_assembly_request_prepare_byname (&req, mono_image_get_alc (image));
req.requesting_assembly = image->assembly;
//req.no_postload_search = TRUE; // FIXME: should this be set?
reference = mono_assembly_request_byname (&aname, &req, NULL);

if (reference == NULL){
char *extra_msg;

Expand Down Expand Up @@ -1602,8 +1599,10 @@ mono_assembly_request_open (const char *filename, const MonoAssemblyOpenRequest
loaded_from_bundle = image != NULL;
}

if (!image)
image = mono_image_open_a_lot (load_req.alc, fname, status);
if (!image) {
MonoImageOpenOptions options = {0, };
image = mono_image_open_a_lot (load_req.alc, fname, status, &options);
}

if (!image){
if (*status == MONO_IMAGE_OK)
Expand Down
13 changes: 12 additions & 1 deletion src/mono/mono/metadata/image-internals.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,17 @@
#include <mono/metadata/image.h>
#include <mono/metadata/loader-internals.h>

typedef struct {
int dont_care_about_cli : 1;
Copy link
Member

@lateralusX lateralusX Jun 7, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any specific reason why these parameters where inverted compared to the arguments passed to functions below?

Copy link
Member

@lateralusX lateralusX Jun 7, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any particular win to use bit fields for this? I think it would be much clearer to use bool type instead, it would also make the init code more clear.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any specific reason why these parameters where inverted compared to the arguments passed to functions below?

Turns out most of the time we care about cli and pecoff, so the callers become a bit simpler: they can just initialize the options struct to zero.

Any particular win to use bit fields for this? I think it would be much clearer to use bool type instead, it would also make the init code more clear.

I think I suggested bitfields more out of habit than any rational reason. These could be booleans.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I'll modify these to a boolean type in the upcoming PR

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(the MonoImage:not_executable we do want to keep in a bitfield - we try to keep the size of that structure down)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is now in PR #71657 @lateralusX @lambdageek

int dont_care_about_pecoff : 1;
} MonoImageLoadOptions;

typedef struct {
MonoImageLoadOptions load_options;
int not_executable : 1;
int metadata_only : 1;
} MonoImageOpenOptions;

MonoImage*
mono_image_loaded_internal (MonoAssemblyLoadContext *alc, const char *name);

Expand All @@ -19,6 +30,6 @@ MonoImage*
mono_image_load_module_checked (MonoImage *image, int idx, MonoError *error);

MonoImage *
mono_image_open_a_lot (MonoAssemblyLoadContext *alc, const char *fname, MonoImageOpenStatus *status);
mono_image_open_a_lot (MonoAssemblyLoadContext *alc, const char *fname, MonoImageOpenStatus *status, const MonoImageOpenOptions *options);

#endif /* __MONO_METADATA_IMAGE_INTERNALS_H__ */
56 changes: 33 additions & 23 deletions src/mono/mono/metadata/image.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ mono_images_unlock(void)
}

static MonoImage *
mono_image_open_a_lot_parameterized (MonoLoadedImages *li, MonoAssemblyLoadContext *alc, const char *fname, MonoImageOpenStatus *status);
mono_image_open_a_lot_parameterized (MonoLoadedImages *li, MonoAssemblyLoadContext *alc, const char *fname, MonoImageOpenStatus *status, const MonoImageOpenOptions *options);

/* Maps string keys to MonoImageStorage values.
*
Expand Down Expand Up @@ -1174,8 +1174,7 @@ dump_encmap (MonoImage *image)
}

static MonoImage *
do_mono_image_load (MonoImage *image, MonoImageOpenStatus *status,
gboolean care_about_cli, gboolean care_about_pecoff)
do_mono_image_load (MonoImage *image, MonoImageOpenStatus *status, const MonoImageLoadOptions *options)
{
ERROR_DECL (error);
GSList *l;
Expand All @@ -1201,7 +1200,7 @@ do_mono_image_load (MonoImage *image, MonoImageOpenStatus *status,
if (status)
*status = MONO_IMAGE_IMAGE_INVALID;

if (care_about_pecoff == FALSE)
if (options->dont_care_about_pecoff == TRUE)
goto done;

if (!mono_image_load_pe_data (image))
Expand All @@ -1210,7 +1209,7 @@ do_mono_image_load (MonoImage *image, MonoImageOpenStatus *status,
image->loader = (MonoImageLoader*)&pe_loader;
}

if (care_about_cli == FALSE) {
if (options->dont_care_about_cli == TRUE) {
goto done;
}

Expand Down Expand Up @@ -1409,8 +1408,7 @@ mono_image_storage_new_raw_data (char *datac, guint32 data_len, gboolean raw_dat
}

static MonoImage *
do_mono_image_open (MonoAssemblyLoadContext *alc, const char *fname, MonoImageOpenStatus *status,
gboolean care_about_cli, gboolean care_about_pecoff, gboolean metadata_only)
do_mono_image_open (MonoAssemblyLoadContext *alc, const char *fname, MonoImageOpenStatus *status, const MonoImageOpenOptions *options)
{
MonoCLIImageInfo *iinfo;
MonoImage *image;
Expand All @@ -1424,6 +1422,7 @@ do_mono_image_open (MonoAssemblyLoadContext *alc, const char *fname, MonoImageOp
}

image = g_new0 (MonoImage, 1);
image->ref_count = 1;
image->storage = storage;
mono_image_init_raw_data (image, storage);
if (!image->raw_data) {
Expand All @@ -1433,14 +1432,14 @@ do_mono_image_open (MonoAssemblyLoadContext *alc, const char *fname, MonoImageOp
*status = MONO_IMAGE_IMAGE_INVALID;
return NULL;
}
iinfo = g_new0 (MonoCLIImageInfo, 1);
image->image_info = iinfo;
image->not_executable = !!options->not_executable;
image->metadata_only = !!options->metadata_only;
image->name = mono_path_resolve_symlinks (fname);
image->filename = g_strdup (image->name);
image->metadata_only = !!metadata_only;
image->ref_count = 1;
iinfo = g_new0 (MonoCLIImageInfo, 1);
image->image_info = iinfo;
image->alc = alc;
return do_mono_image_load (image, status, care_about_cli, care_about_pecoff);
return do_mono_image_load (image, status, &options->load_options);
}

/**
Expand Down Expand Up @@ -1627,7 +1626,8 @@ mono_image_open_from_data_internal (MonoAssemblyLoadContext *alc, char *data, gu
image->ref_count = 1;
image->alc = alc;

image = do_mono_image_load (image, status, TRUE, TRUE);
MonoImageLoadOptions options = {0, };
image = do_mono_image_load (image, status, &options);
if (image == NULL)
return NULL;

Expand Down Expand Up @@ -1742,7 +1742,8 @@ mono_image_open_from_module_handle (MonoAssemblyLoadContext *alc, HMODULE module
image->ref_count = has_entry_point ? 0 : 1;
image->alc = alc;

image = do_mono_image_load (image, status, TRUE, TRUE);
MonoImageLoadOptions options = {0, };
image = do_mono_image_load (image, status, &options);
if (image == NULL)
return NULL;

Expand All @@ -1761,11 +1762,12 @@ mono_image_open_full (const char *fname, MonoImageOpenStatus *status, gboolean r
*status = MONO_IMAGE_NOT_SUPPORTED;
return NULL;
}
return mono_image_open_a_lot (mono_alc_get_default (), fname, status);
MonoImageOpenOptions options = {0, };
return mono_image_open_a_lot (mono_alc_get_default (), fname, status, &options);
}

static MonoImage *
mono_image_open_a_lot_parameterized (MonoLoadedImages *li, MonoAssemblyLoadContext *alc, const char *fname, MonoImageOpenStatus *status)
mono_image_open_a_lot_parameterized (MonoLoadedImages *li, MonoAssemblyLoadContext *alc, const char *fname, MonoImageOpenStatus *status, const MonoImageOpenOptions *options)
{
MonoImage *image;
GHashTable *loaded_images = mono_loaded_images_get_hash (li);
Expand Down Expand Up @@ -1867,18 +1869,18 @@ mono_image_open_a_lot_parameterized (MonoLoadedImages *li, MonoAssemblyLoadConte
mono_images_unlock ();

// Image not loaded, load it now
image = do_mono_image_open (alc, fname, status, TRUE, TRUE, FALSE);
image = do_mono_image_open (alc, fname, status, options);
if (image == NULL)
return NULL;

return register_image (li, image);
}

MonoImage *
mono_image_open_a_lot (MonoAssemblyLoadContext *alc, const char *fname, MonoImageOpenStatus *status)
mono_image_open_a_lot (MonoAssemblyLoadContext *alc, const char *fname, MonoImageOpenStatus *status, const MonoImageOpenOptions *options)
{
MonoLoadedImages *li = mono_alc_get_loaded_images (alc);
return mono_image_open_a_lot_parameterized (li, alc, fname, status);
return mono_image_open_a_lot_parameterized (li, alc, fname, status, options);
}

/**
Expand All @@ -1893,7 +1895,8 @@ mono_image_open_a_lot (MonoAssemblyLoadContext *alc, const char *fname, MonoImag
MonoImage *
mono_image_open (const char *fname, MonoImageOpenStatus *status)
{
return mono_image_open_a_lot (mono_alc_get_default (), fname, status);
MonoImageOpenOptions options = {0, };
return mono_image_open_a_lot (mono_alc_get_default (), fname, status, &options);
}

/**
Expand All @@ -1911,7 +1914,9 @@ mono_pe_file_open (const char *fname, MonoImageOpenStatus *status)
{
g_return_val_if_fail (fname != NULL, NULL);

return do_mono_image_open (mono_alc_get_default (), fname, status, FALSE, TRUE, FALSE);
MonoImageOpenOptions options = {0, };
options.load_options.dont_care_about_cli = 1;
return do_mono_image_open (mono_alc_get_default (), fname, status, &options);
}

/**
Expand All @@ -1926,7 +1931,10 @@ mono_image_open_raw (MonoAssemblyLoadContext *alc, const char *fname, MonoImageO
{
g_return_val_if_fail (fname != NULL, NULL);

return do_mono_image_open (alc, fname, status, FALSE, FALSE, FALSE);
MonoImageOpenOptions options = {0, };
options.load_options.dont_care_about_cli = 1;
options.load_options.dont_care_about_pecoff = 1;
return do_mono_image_open (alc, fname, status, &options);
}

/*
Expand All @@ -1937,7 +1945,9 @@ mono_image_open_raw (MonoAssemblyLoadContext *alc, const char *fname, MonoImageO
MonoImage *
mono_image_open_metadata_only (MonoAssemblyLoadContext *alc, const char *fname, MonoImageOpenStatus *status)
{
return do_mono_image_open (alc, fname, status, TRUE, TRUE, TRUE);
MonoImageOpenOptions options = {0, };
options.metadata_only = 1;
return do_mono_image_open (alc, fname, status, &options);
}

/**
Expand Down
3 changes: 3 additions & 0 deletions src/mono/mono/metadata/metadata-internals.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,9 @@ struct _MonoImage {
/* Whenever this is a dynamically emitted module */
guint8 dynamic : 1;

/* Whenever this image is not an executable, such as .mibc */
guint8 not_executable : 1;

/* Whenever this image contains uncompressed metadata */
guint8 uncompressed_metadata : 1;

Expand Down
Loading