ocaml / dune

A composable build system for OCaml.
https://dune.build/
MIT License
1.6k stars 395 forks source link

flambda with -nostdlib and transtive stdlib not finding cmx #4039

Open EduardoRFS opened 3 years ago

EduardoRFS commented 3 years ago

I found a situation where flambda fails to optimize a project when using dune, transitive stdlib and -nostdlib. I’ve included reproduction steps and a proposed fix.

Context

The goal is to compile Tezos using dune with the flambda optimization enabled. Flambda relies on either the build system (dune) or the compiler to provide the position of cmx files; however, dune assumes that the compiler will provide the position of the stdlib. When using the -nostdlib flag, this is not the case. Thus, flambda cannot find them and fails to optimize, giving the Warning 58.

Expected behavior

Dune should add every dependency during the linking of an executable, including transitive dependencies and the stdlib. Flambda should behave identically to the closure compiler in this regard.

I propose the stdlib be added during the linking of every executable.

Actual behavior

Dune seems not to add the stdlib when it is only included as a transitive dependency. For example, in the dependency tree depicted below, the compiler cannot find the cmx files:

custom_stdlib(stdlib) -> lib_a(nostdlib) -> executable(stdlib)

Reproduction

You can try the following the reproduction with and without flambda when using --profile=release. Without flambda, it will compile without any warning. With flambda enabled, it still builds; however, it fails to optimize as the cmx for the stdlib isn’t available.

https://github.com/EduardoRFS/dune-flambda-nostdlib-no-cmx

Specifications

nojb commented 3 years ago

You are passing -nostdlib. Could you clarify what behaviour would you like in this situation?

EduardoRFS commented 3 years ago

@nojb I'm passing -nostdlib for a library, as you can see in the example, lib_a is using -nostdlib but custom_stdlib isn't, and lib_a depends on custom_stdlib so lib_a should have access to the stdlib .cmx but not the the .cmi

The problem goes away as soon as the executable has a direct reference for the stdlib on the executable.

EduardoRFS commented 3 years ago

I also updated the description to include more detail, but the reproduction is simple, try it with mainline vs flambda and you're going to notice the problem.

edit:

I tried to make myself even clearer and added a bit more context on why this matters

nojb commented 3 years ago

If I'm understanding correctly, you would like to make stdlib.cmx visible to the compiler but not stdlib.cmi. This is not currently possible to do, and would need some work at the compiler level to achieve.

EduardoRFS commented 3 years ago

@nojb stdlib.cmx is only is only needed for optimizations during the linking of an executable or library, it's not needed to build the actual module. That's the problem. There is no problem to add the full folder during linking, as .cmi will not even be used

edit:

That seems to be wrong the .cmx file is needed for the build of the actual module and that can be found here

https://github.com/EduardoRFS/dune-flambda-nostdlib-no-cmx/blob/main/build.output.txt#L120

nojb commented 3 years ago

Yes, the .cmx is used during compilation to perform inlining. Perhaps dune could help in some way, but that would require dune having a higher-level understanding of what you are trying to achieve using -nostdlib.

For your use-case in particular, couldn't you play with shadowing to achieve an equivalent effect, eg by opening a module with a top-level Stdlib submodule?

EduardoRFS commented 3 years ago

@nojb I will give some tries, maybe we can still get the optimization with some dune rules to copy things. That seems to be a big problem if someone wants to use (implicit_transitive_deps false)

EduardoRFS commented 3 years ago

Tried (implicit_transitive_deps false) on a private project and it's exactly that, a lot of inlining oportunities are lost because there is no cmx file available.

Seems like the compiler needs a -L.

mobileink commented 2 years ago

Yes, the .cmx is used during compilation to perform inlining.

Are you sure about that? I would expect such optimizations to occur only at link time. If they're done during compilation you'd end up repeating the same optimization.

If the .cmx is missing the compiler just issues a warning, which I take to mean: "you have not provided a .cmx for this guy, but you have also not disabled cross-module optimization (you did not pass -opaque), so if you're expecting cross-module optimization at link time you're probably going to be disappointed."

But a clever build system should be able to omit cmx deps at compile time while providing them at link time.