fix compile=all, move code related to static compilation to precompile.c #20820

JeffBezanson · 2017-02-27T00:17:56Z

More moving stuff around. I think it will be useful to have a dedicated file for static-compilation-related stuff.

vtjnash

compileout doesn't seem like the best name choice

vtjnash · 2017-02-27T01:03:48Z

src/compileout.c

+    return jl_options.outputo || jl_options.outputbc || jl_options.outputji;
+}
+
+void jl_precompile(int all);


should be in a header file

vtjnash · 2017-02-27T01:05:05Z

src/compileout.c

+    JL_GC_POP();
+}
+
+int jl_is_rettype_inferred(jl_method_instance_t *li);


should be in a header also

vtjnash · 2017-02-27T01:05:07Z

src/compileout.c

+    return 1;
+}
+
+#if 0  // TODO restore this for jb/subtype


probably should be restored, in order to make sure this code movement isn't breaking anything here

Ok, I'll do that as part of this PR.

JeffBezanson · 2017-02-27T03:02:36Z

Suggest a name?

vtjnash · 2017-02-27T18:59:00Z

This seems pretty tightly coupled to the rest of gf.c to me. I'm not sure this code movement is worthwhile.

JeffBezanson · 2017-02-27T19:18:28Z

This code does something pretty specific: traversing modules to find everything that needs to be compiled for compile=all. In theory, if you were never going to use compile=all you could exclude this code from the build. Sure it uses Method and MethodInstance a fair amount, but so do several other files. gf.c is also coupled to typemap.c, but they can quite sensibly be separate files since typemap.c implements a data structure and gf.c mostly implements caching algorithms on top of it.

vtjnash · 2017-02-27T19:32:22Z

TypeMap is a fairly general concept though, and can get reused elsewhere. The compile-all code is pretty tightly coupled with the precompile and caching logic. Sure, doesn't mean this can't move, it just means I don't have a clear idea of what categorization this should have. Maybe precompile.c would be a better name?

JeffBezanson · 2017-02-27T19:46:27Z

precompile.c is fine with me. It would be nice if we could move some of the module precompile code there eventually, to make a single integrated binary-artifact-generator, but that code really is quite coupled with dump.c.

I would say this code is anti-reusable: it's not used by anything else in the system, and is actually an application of various reusable mechanisms that exist elsewhere, such as looking up specializations and traversing typemaps. I'm looking toward the idea of a juliac program separable from the julia runtime, where we want the runtime to consist of the minimal stuff needed to run any julia code.

JeffBezanson · 2017-02-28T17:41:41Z

Ok, I think I have compile=all working.

tkelman · 2017-02-28T18:47:46Z

CI will be broken until the S3 outage is resolved.

vtjnash · 2017-02-28T20:15:32Z

src/jltypes.c

+    return jl_count_union_components(u->a) + jl_count_union_components(u->b);
+}
+
+static jl_value_t *nth_union_component(jl_value_t *v, int *pi)


needs comments

vtjnash · 2017-02-28T20:22:09Z

src/precompile.c

+        m->unspecialized = linfo;
+        jl_gc_wb(m, linfo);
+        if (linfo->jlcall_api == 2)
+            continue;


don't uncomment this - this is completely invalid right now.

Why is that?

Should I set src = m->source instead?

the unspecialized field is not world-aware, so it can't store world-specialized data. Yes, use src = m->source.

vtjnash · 2017-02-28T20:23:12Z

src/precompile.c

+        // keep track of whether all possible signatures have been cached (and thus whether it can skip trying to compile the template function)
+        // this is necessary because many intrinsics try to call static_eval and thus are not compilable unspecialized
+        int complete = _compile_all_union((jl_value_t*)ml->sig);
+        if (complete) {


can delete this branch test now (the comment about intrinsics calling static_eval is incorrect)

Yes you're right about static_eval, but isn't it still true that we don't need to compile an unspecialized method if the signature is a leaf type or union of leaf types?

That's going to depend on how we want to define the behavior of the compile=all output. Branching on isleaftype is also valid. I guess the main point is just that this is now just an optimization (while before it was a crutch that was needed for handling intrinsics).

vtjnash · 2017-02-28T20:24:22Z

src/precompile.c

+        else {
+            jl_compile_linfo(&linfo, src, jl_world_counter, &jl_default_cgparams);
+            assert(linfo->functionObjectsDecls.functionObject != NULL);
+            m->unspecialized = linfo;


this'll corrupt the system. don't store an inferred object here.

vtjnash · 2017-02-28T20:26:06Z

src/precompile.c

+    jl_printf(JL_STDERR, "\n");
+}
+
+static void foreach_def_in_module(jl_module_t *m, int (*visit)(jl_typemap_entry_t *, void*), jl_array_t *out)


visit should be a jl_typemap_visitor_fptr type

JeffBezanson · 2017-03-01T23:53:21Z

Comments addressed. However, this doesn't seem to quite do what we want. If I try building a system image with this, I still get lots of code missing warnings with compile=no. What am I doing wrong?

JeffBezanson · 2017-03-17T18:32:09Z

Adding to milestone since we need to fix compile=all for the release.

vtjnash · 2017-03-23T17:16:30Z

this gets it much closer for me:

diff --cc src/gf.c
index 5651e279d1,e07378ca58..0000000000
--- a/src/gf.c
+++ b/src/gf.c
diff --git a/base/interactiveutil.jl b/base/interactiveutil.jl
index 1d569265a3..12663bb923 100644
--- a/base/interactiveutil.jl
+++ b/base/interactiveutil.jl
@@ -61,9 +61,10 @@ function edit(path::AbstractString, line::Integer=0)
     end
 
     if is_windows() && name == "open"
-        systemerror(:edit, ccall((:ShellExecuteW,"shell32"), stdcall, Int,
-                                 (Ptr{Void}, Cwstring, Cwstring, Ptr{Void}, Ptr{Void}, Cint),
-                                 C_NULL, "open", path, C_NULL, C_NULL, 10) ≤ 32)
+        @static is_windows() && # don't emit this ccall on other platforms
+            systemerror(:edit, ccall((:ShellExecuteW, "shell32"), stdcall, Int,
+                                     (Ptr{Void}, Cwstring, Cwstring, Ptr{Void}, Ptr{Void}, Cint),
+                                     C_NULL, "open", path, C_NULL, C_NULL, 10) ≤ 32)
     elseif background
         spawn(pipeline(cmd, stderr=STDERR))
     else

diff --git a/src/dump.c b/src/dump.c
index dc9a97ed79..5d6bc80aa3 100644
--- a/src/dump.c
+++ b/src/dump.c
@@ -422,7 +422,15 @@ static void jl_update_all_fptrs(void)
 {
     //jl_printf(JL_STDOUT, "delayed_fptrs_n: %d\n", delayed_fptrs_n);
     void **fvars = sysimg_fvars;
-    if (fvars == 0) return;
+    if (fvars == NULL) {
+        size_t i;
+        for (i = 0; i < delayed_fptrs_n; i++) {
+            jl_method_instance_t *li = delayed_fptrs[i].li;
+            assert(li->jlcall_api && li->jlcall_api != 2);
+            li->jlcall_api = 0;
+        }
+        return;
+    }
     // jl_fptr_to_llvm needs to decompress some ASTs, therefore this needs to be NULL
     // to skip trying to restore GlobalVariable pointers in jl_deserialize_gv
     sysimg_gvars = NULL;
@@ -431,7 +439,7 @@ static void jl_update_all_fptrs(void)
     jl_method_instance_t **linfos = (jl_method_instance_t**)malloc(sizeof(jl_method_instance_t*) * sysimg_fvars_max);
     for (i = 0; i < delayed_fptrs_n; i++) {
         jl_method_instance_t *li = delayed_fptrs[i].li;
-        assert(li->def);
+        assert(li->def && li->jlcall_api && li->jlcall_api != 2);
         int32_t cfunc = delayed_fptrs[i].cfunc - 1;
         if (cfunc >= 0) {
             jl_fptr_to_llvm((jl_fptr_t)fvars[cfunc], li, 1);

diff --git a/src/precompile.c b/src/precompile.c
index 909e180b5d..e34ff9a81b 100644
--- a/src/precompile.c
+++ b/src/precompile.c
@@ -266,14 +266,14 @@ static void _compile_all_deq(jl_array_t *found)
         int complete = _compile_all_union((jl_value_t*)ml->sig);
         // if _compile_all_union returns 1, all possible signatures have been covered, so we don't
         // need to compile an unspecialized version.
-        if (complete) {
+        if (0) {
             if (linfo->fptr == NULL && linfo->functionObjectsDecls.functionObject == NULL)
                 // indicate that this method doesn't need to be compiled, because it was fully covered above
                 // TODO: do this some other way
                 linfo->fptr = (jl_fptr_t)(uintptr_t)-1;
         }
         else {
-            jl_compile_linfo(&linfo, src, jl_world_counter, &jl_default_cgparams);
+            jl_compile_linfo(&linfo, src, linfo->min_world, &jl_default_cgparams);
             assert(linfo->functionObjectsDecls.functionObject != NULL);
         }
     }

vtjnash · 2017-04-04T14:26:07Z

Should be entirely fixed now.

JeffBezanson · 2017-04-04T14:54:45Z

src/precompile.c

+        //    continue;
+
+        _compile_all_union((jl_value_t*)ml->sig);
+        // if _compile_all_union returns 1, all possible signatures have been covered, so we don't


Is this comment not true any more?

Not really, no. The main culprit is inference, which runs in its own world. But it was also only ever intended as a work-around hack for the duct tape, so it's simpler to just remove this now that intrinsics no longer require this.

I don't quite get it. I thought the idea of the return value of _compile_all_union was that we didn't need to infer or compile an unspecialized version, since the signature is equivalent to a set of concrete signatures.

JeffBezanson · 2017-04-04T14:56:01Z

src/precompile.c

+    jl_typemap_visitor(mt->defs, compile_all_enq__, env);
+}
+
+void jl_foreach_mtable_in_module(


Where is this defined?

also fix compile=all for jb/subtype

timholy · 2017-04-06T16:03:07Z

Thanks folks! (Ref #21141)

vtjnash reviewed Feb 27, 2017

View reviewed changes

JeffBezanson force-pushed the jb/reorg3 branch from 99cc0b4 to 0852fd3 Compare February 28, 2017 17:38

JeffBezanson changed the title ~~reorg: move code related to static compilation to compileout.c~~ reorg: move code related to static compilation to compileout.c, fix compile=all Feb 28, 2017

JeffBezanson force-pushed the jb/reorg3 branch from 0852fd3 to e2c17fd Compare February 28, 2017 17:44

vtjnash reviewed Feb 28, 2017

View reviewed changes

JeffBezanson force-pushed the jb/reorg3 branch from e2c17fd to 6edd122 Compare March 1, 2017 23:51

JeffBezanson changed the title ~~reorg: move code related to static compilation to compileout.c, fix compile=all~~ fix compile=all, move code related to static compilation to precompile.c Mar 6, 2017

vtjnash mentioned this pull request Mar 7, 2017

new subtyping issues #19998

Closed

53 tasks

JeffBezanson added this to the 0.6.0 milestone Mar 17, 2017

vtjnash force-pushed the jb/reorg3 branch from 6edd122 to 7d19a23 Compare April 4, 2017 01:50

JeffBezanson commented Apr 4, 2017

View reviewed changes

JeffBezanson and others added 2 commits April 5, 2017 17:37

reorg: move code related to static compilation to precompile.c

74334d4

also fix compile=all for jb/subtype

improve backedge debugging to list causes

beea404

vtjnash force-pushed the jb/reorg3 branch from 7d19a23 to beea404 Compare April 5, 2017 21:38

vtjnash merged commit 5cacdd6 into master Apr 6, 2017

vtjnash deleted the jb/reorg3 branch April 6, 2017 15:37

timholy mentioned this pull request Apr 6, 2017

World Age JuliaGizmos/Interact.jl#161

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix compile=all, move code related to static compilation to precompile.c #20820

fix compile=all, move code related to static compilation to precompile.c #20820

JeffBezanson commented Feb 27, 2017 •

edited by vtjnash

Loading

vtjnash left a comment

vtjnash Feb 27, 2017

vtjnash Feb 27, 2017

vtjnash Feb 27, 2017

JeffBezanson Feb 27, 2017

JeffBezanson commented Feb 27, 2017

vtjnash commented Feb 27, 2017

JeffBezanson commented Feb 27, 2017

vtjnash commented Feb 27, 2017

JeffBezanson commented Feb 27, 2017

JeffBezanson commented Feb 28, 2017

tkelman commented Feb 28, 2017

vtjnash Feb 28, 2017

vtjnash Feb 28, 2017

JeffBezanson Feb 28, 2017

vtjnash Feb 28, 2017

vtjnash Feb 28, 2017

JeffBezanson Feb 28, 2017

vtjnash Feb 28, 2017

vtjnash Feb 28, 2017

vtjnash Feb 28, 2017

JeffBezanson commented Mar 1, 2017

JeffBezanson commented Mar 17, 2017

vtjnash commented Mar 23, 2017

vtjnash commented Apr 4, 2017

JeffBezanson Apr 4, 2017

vtjnash Apr 4, 2017

JeffBezanson Apr 4, 2017

JeffBezanson Apr 4, 2017

vtjnash Apr 4, 2017

timholy commented Apr 6, 2017

fix compile=all, move code related to static compilation to precompile.c #20820

fix compile=all, move code related to static compilation to precompile.c #20820

Conversation

JeffBezanson commented Feb 27, 2017 • edited by vtjnash Loading

vtjnash left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JeffBezanson commented Feb 27, 2017

vtjnash commented Feb 27, 2017

JeffBezanson commented Feb 27, 2017

vtjnash commented Feb 27, 2017

JeffBezanson commented Feb 27, 2017

JeffBezanson commented Feb 28, 2017

tkelman commented Feb 28, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JeffBezanson commented Mar 1, 2017

JeffBezanson commented Mar 17, 2017

vtjnash commented Mar 23, 2017

vtjnash commented Apr 4, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timholy commented Apr 6, 2017

JeffBezanson commented Feb 27, 2017 •

edited by vtjnash

Loading