Re: MPS: Please check if scratch/igc builds with native compilation

[Gerd Möllmann]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> Helmut Eller <eller.helmut@gmail.com> writes:
>
>>> I've checked that d_reloc is indeed scanned by fix_comp_unit. The
>>> check gives me reasonable confidence that this "should work". But as
>>> an alternative, I also made all the things like d_reloc in the .elns
>>> ambiguous roots, so that they cannot possibly be moved, if all works as
>>> expected.
>>
>> Registering the dump as root happens rather late.  The relocation code
>> allocates a hash table and stores a reference to it in
>> comp_u->lambda_gc_guard_h.  By that time the dump should already be a
>> root.  Can we register the dump earlier?  AFAIU, the dumper writes zeros
>> in the cells for to-be-relocated references and the scan code will
>> ignore them.  So I think this could work:
>>
>> diff --git a/src/pdumper.c b/src/pdumper.c
>> index b039e375c1f..39484a16c95 100644
>> --- a/src/pdumper.c
>> +++ b/src/pdumper.c
>> @@ -5958,6 +5958,7 @@ pdumper_load (const char *dump_filename, char *argv0)
>>         & ~(DUMP_ALIGNMENT - 1));
>>    void *hot_start = (void *) (dump_base + aligned_header_size);
>>    void *hot_end = (void *) (dump_base + header->discardable_start);
>> +  igc_on_pdump_loaded (hot_start, hot_end);
>>  #endif
>
> AFAIU, at the point above the necessary relocations haven't yet been
> done, so the dump is still in its "raw" form as it is in the file. Which
> means, among other things, that Lisp_Objects haven't been changed to
> point to where the dump is mmap'd or where the data segment of Emacs is
> and so on. So I don't think that would work.
>
>>    dump_do_all_dump_reloc_for_phase (header, dump_base, EARLY_RELOCS);
>> @@ -6002,10 +6003,6 @@ pdumper_load (const char *dump_filename, char *argv0)
>>    dump_private.load_time = timespectod (load_timespec);
>>    dump_private.dump_filename = dump_filename_copy;
>>
>> -# ifdef HAVE_MPS
>> -  igc_on_pdump_loaded (hot_start, hot_end);
>> -# endif
>> -
>>   out:
>>    for (int i = 0; i < ARRAYELTS (sections); ++i)
>>      dump_mmap_release (&sections[i]);
>
> But, what if we park MPS while we are loading the dump? WDYT?

That would be something like the below. Alas, it doesn't change the
asserts in the native comp build.


Unstaged
modified   lisp/emacs-lisp/comp.el
@@ -54,7 +54,7 @@ comp
   "Emacs Lisp native compiler."
   :group 'lisp)
 
-(defcustom native-comp-speed 2
+(defcustom native-comp-speed 0
   "Optimization level for native compilation, a number between -1 and 3.
  -1 functions are kept in bytecode form and no native compilation is performed
     (but *.eln files are still produced, and include the compiled code in
@@ -68,7 +68,7 @@ native-comp-speed
   :safe #'integerp
   :version "28.1")
 
-(defcustom native-comp-debug 0
+(defcustom native-comp-debug 2
   "Debug level for native compilation, a number between 0 and 3.
 This is intended for debugging the compiler itself.
   0 no debug output.
modified   src/igc.c
@@ -2281,6 +2281,18 @@ igc_park_arena (void)
   return count;
 }
 
+void
+igc_park (void)
+{
+  mps_arena_park (global_igc->arena);
+}
+
+void
+igc_release (void)
+{
+  mps_arena_release (global_igc->arena);
+}
+
 static igc_root_list *
 root_find (void *start)
 {
modified   src/igc.h
@@ -121,6 +121,8 @@ #define EMACS_IGC_H
 size_t igc_hash (Lisp_Object key);
 void igc_create_charset_root (void *table, size_t size);
 specpdl_ref igc_park_arena (void);
+void igc_park (void);
+void igc_release (void);
 void igc_check_vector (const struct Lisp_Vector *v);
 void igc_postmortem (void);
 void igc_on_grow_specpdl (void);
modified   src/pdumper.c
@@ -5809,6 +5809,10 @@ pdumper_load (const char *dump_filename, char *argv0)
   /* We can load only one dump.  */
   eassert (!dump_loaded_p ());
 
+#ifdef HAVE_MPS
+  igc_park ();
+# endif
+
   int err;
   int dump_fd = emacs_open_noquit (dump_filename, O_RDONLY, 0);
   if (dump_fd < 0)
@@ -5980,6 +5984,9 @@ pdumper_load (const char *dump_filename, char *argv0)
   if (dump_fd >= 0)
     emacs_close (dump_fd);
 
+#ifdef HAVE_MPS
+  igc_release ();
+#endif
   return err;
 }