-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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.
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
Provide the option to bootstrap FlexDLL with OCaml #388
Conversation
6b741d4
to
bd271f1
Compare
Please note that Travis is failing because of the submodule (which yields the question of why the CI is doing a submodule init, given that there were no other submodules) |
bd271f1
to
9b62404
Compare
Travis build fixed |
An alternative approach to break the dependency cycle between ocaml and flexlink would be to only simplify building OCaml without flexlink (I think @damiendoligez does it regularly with the cygwin port), and add to the flexdll build system the required stuff to compile a local OCaml only to build flexlink itself. This would reduce the impact on OCaml build system. And it would allow building flexlink more naturally with a different port than the one you want to use for OCaml itself; for instance, to create a 64-bit version of flexlink to allow it to work on larger bigger object files, even if you want to use a 32-bit port of OCaml; or, to create a version of flexlink compiled with the Cygwin port of OCaml so that it does not need to call the external cygpath command (which would probably give a significant speedup). |
Or we could do both 😄 Indeed, copying over the required support for the Using, for example, a Cygwin flexlink with a native Windows port falls neatly into work I'm planning for later this year with Windows cross-compiling anyhow - again, for OPAM, I'd like to be able to produce:
(which also gives the side-goal of accessing Microsoft's ARM compilers, so I can get OCaml programs running on Windows 10 IoT!) In all that, I'd find it natural and preferable still to have OCaml's build process able to bootstrap its required FlexDLL, but there is of course no bootstrapping argument for that - it's just a (big) convenience. |
I've pushed the first two commits to trunk. Let me think more about the rest! |
9b62404
to
4fcbecf
Compare
I've just revised things slightly - in running my own testing, I hit a build-tree usability issue where I'd re-built a rebuilt bytecode flexlink but because ocamlopt.opt was still floating around the testsuite would attempt to invoke flexlink directly (i.e. without using ocamlrun) and so not work |
I'm unsurprisingly eager for this to be considered for 4.03.0 - have you had time to think further on it? |
@@ -105,7 +105,7 @@ install:: | |||
clean:: | |||
rm -f ocamlmklib | |||
|
|||
ocamlmklibconfig.ml: ../config/Makefile | |||
ocamlmklibconfig.ml:: ../config/Makefile |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this change related to this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes - because of the addition in tools/Makefile.nt
because I need an extra line at the end of ocamlmklibconfig.ml
for Windows only. You can't mix :
and ::
rules. This has the intentional effect of executing the instructions in Makefile.shared
followed by the appending to that file in Makefile.nt
(the clean
targets in many of the Makefile
s are constructed in the same way).
I'd prefer to minimize the diff between Makefile and Makefile.nt. As long as common parts are not shared, it is useful to reduce the diff to help spot problems; and to allow sharing, it's good to avoid invasive changes such as the addition of REQUIRE_FLEXDLL as a dependency to many rules. I don't think that all rules are supposed to be usable in arbitrary order. What do you think of including this requirement only to more specific places. Or, alternatively, to add a more pre-compilation requirement target (with a more generic name) also to Unix's Makefile (which would be empty there)? |
I'd happily go along with removing the |
I'm in favor of everything which still suits your needs while making the diff as small as possible and reducing divergence between Windows and non-Windows Makefiles. |
Ok - I'll have a go at reducing the diff as far as possible. Thanks for reviewing it! |
4fcbecf
to
1c0836d
Compare
OK, here we go! Rebased on to this morning's
I don't think I can sensibly make the diff too much smaller - what do you think of the revised version? |
Allows the four native Windows ports to compile FlexDLL and flexlink "in-tree" by placing the sources in directory `flexdll` and issuing `make -f Makefile.nt flexdll` FlexDLL must still be installed somewhere in PATH before OCaml itself can be compiled. The `flexdll` target leaves the OCaml build tree in a sufficiently clean state to allow `world` to be run after installation of FlexDLL.
`install-flexdll` target added to Makefile.nt which installs flexlink.exe and the correct objects, manifests and flexdll.h to `$(INSTALL_BINDIR)`. When this target has been used, the behaviour of opt.opt is altered to compile a native-code version of flexlink.exe. This is subsequently installed by the install target. This means that Windows can be fully built by issuing: make -f Makefile.nt flexdll install-flexdll world opt.opt install
Done! |
Cool - I'm just testing a rebase onto current trunk so that Travis can comment on this... |
Messages for obtaining flexdll sources updated in Makefile.nt
OCAML_FLEXLINK is inspected by ocamlopt, ocamlc and ocamlmklib and allows the flexlink command to be overriden. This is primarily intended as a solution for bootstrapping OCaml with FlexDLL, thus allowing a bytecode image of flexlink launched with ocamlrun to be executed, instead of requiring flexlink to be in PATH.
Extend the previous patch allowing make -f Makefile.nt flexdll install-flexdll not to require the install-flexdll stage. OCAML_FLEXLINK is utilised as required to allow compilation of the entire system using an in-tree compiled flexlink. The build process simply required the flexdll target to appear before world. opt.opt compiles a native code version of flexlink.exe as flexlink.opt. install always installs flexlink.exe if it was compiled along with any required .manifest files. It also installs the appropriate .o/.obj files to $(INSTALL_LIBDIR). At present, the bootstrapping is not extended to the Cygwin ports.
Remove duplication between tools/ocamlmklibconfig.ml and utils/config.ml and link ocamlmklib with config.cmo from compiler-libs.
1c0836d
to
ae49015
Compare
OK - compilation tested. Native Windows trunk is broken at the moment (PR coming soon...), but this builds. I shall shortly verify that the testsuite is still at full pass, but I have to rebase another branch first (too many open PRs at the moment!) |
And that's a full testsuite pass for MSVC x86 and x64 |
Passing |
Ok, I've fixed the conflict and push the current version to trunk. I'm still interested in keeping the diff between Makefile and Makefile.nt under control as a preparation for more sharing between the two, but this is certainly not blocking. |
Thank you - you've made my Monday 😄 I'll have a go at reducing that diff as you suggest shortly - there will also be a small update wanted to shift the SHA apropos of the two PRs on FlexDLL this morning. |
Just looking at this - is defining MAKERECBOOT necessarily a good idea at this stage? Makefile.nt already has differences here since Makefile uses |
OK, I can't say I understand everything here, but I have a warning: There is an ocaml package in cygwin, which I'm maintaining, and it uses the flexdll package, also in cygwin, which somebody else is maintaining, and which is different from the upstream flexdll in ways that I don't understand. All I know is, if I build the ocaml cygwin package with the regular flexdll (instead of the flexdll provided by cygwin) some things will break. I just want you guys to be aware of this workflow. |
At present the submodule shouldn't interfere in any way - especially at the moment as Cygwin uses the Makefile, not Makefile.nt route. |
23a7f73 flambda-backend: Fix some Debuginfo.t scopes in the frontend (ocaml#248) 33a04a6 flambda-backend: Attempt to shrink the heap before calling the assembler (ocaml#429) 8a36a16 flambda-backend: Fix to allow stage 2 builds in Flambda 2 -Oclassic mode (ocaml#442) d828db6 flambda-backend: Rename -no-extensions flag to -disable-all-extensions (ocaml#425) 68c39d5 flambda-backend: Fix mistake with extension records (ocaml#423) 423f312 flambda-backend: Refactor -extension and -standard flags (ocaml#398) 585e023 flambda-backend: Improved simplification of array operations (ocaml#384) faec6b1 flambda-backend: Typos (ocaml#407) 8914940 flambda-backend: Ensure allocations are initialised, even dead ones (ocaml#405) 6b58001 flambda-backend: Move compiler flag -dcfg out of ocaml/ subdirectory (ocaml#400) 4fd57cf flambda-backend: Use ghost loc for extension to avoid expressions with overlapping locations (ocaml#399) 8d993c5 flambda-backend: Let's fix instead of reverting flambda_backend_args (ocaml#396) d29b133 flambda-backend: Revert "Move flambda-backend specific flags out of ocaml/ subdirectory (ocaml#382)" (ocaml#395) d0cda93 flambda-backend: Revert ocaml#373 (ocaml#393) 1c6eee1 flambda-backend: Fix "make check_all_arches" in ocaml/ subdirectory (ocaml#388) a7960dd flambda-backend: Move flambda-backend specific flags out of ocaml/ subdirectory (ocaml#382) bf7b1a8 flambda-backend: List and Array Comprehensions (ocaml#147) f2547de flambda-backend: Compile more stdlib files with -O3 (ocaml#380) 3620c58 flambda-backend: Four small inliner fixes (ocaml#379) 2d165d2 flambda-backend: Regenerate ocaml/configure 3838b56 flambda-backend: Bump Menhir to version 20210419 (ocaml#362) 43c14d6 flambda-backend: Re-enable -flambda2-join-points (ocaml#374) 5cd2520 flambda-backend: Disable inlining of recursive functions by default (ocaml#372) e98b277 flambda-backend: Import ocaml#10736 (stack limit increases) (ocaml#373) 82c8086 flambda-backend: Use hooks for type tree and parse tree (ocaml#363) 33bbc93 flambda-backend: Fix parsecmm.mly in ocaml subdirectory (ocaml#357) 9650034 flambda-backend: Right-to-left evaluation of arguments of String.get and friends (ocaml#354) f7d3775 flambda-backend: Revert "Magic numbers" (ocaml#360) 0bd2fa6 flambda-backend: Add [@inline ready] attribute and remove [@inline hint] (not [@inlined hint]) (ocaml#351) cee74af flambda-backend: Ensure that functions are evaluated after their arguments (ocaml#353) 954be59 flambda-backend: Bootstrap dd5c299 flambda-backend: Change prefix of all magic numbers to avoid clashes with upstream. c2b1355 flambda-backend: Fix wrong shift generation in Cmm_helpers (ocaml#347) 739243b flambda-backend: Add flambda_oclassic attribute (ocaml#348) dc9b7fd flambda-backend: Only speculate during inlining if argument types have useful information (ocaml#343) aa190ec flambda-backend: Backport fix from PR#10719 (ocaml#342) c53a574 flambda-backend: Reduce max inlining depths at -O2 and -O3 (ocaml#334) a2493dc flambda-backend: Tweak error messages in Compenv. 1c7b580 flambda-backend: Change Name_abstraction to use a parameterized type (ocaml#326) 07e0918 flambda-backend: Save cfg to file (ocaml#257) 9427a8d flambda-backend: Make inlining parameters more aggressive (ocaml#332) fe0610f flambda-backend: Do not cache young_limit in a processor register (upstream PR 9876) (ocaml#315) 56f28b8 flambda-backend: Fix an overflow bug in major GC work computation (ocaml#310) 8e43a49 flambda-backend: Cmm invariants (port upstream PR 1400) (ocaml#258) e901f16 flambda-backend: Add attributes effects and coeffects (#18) aaa1cdb flambda-backend: Expose Flambda 2 flags via OCAMLPARAM (ocaml#304) 62db54f flambda-backend: Fix freshening substitutions 57231d2 flambda-backend: Evaluate signature substitutions lazily (upstream PR 10599) (ocaml#280) a1a07de flambda-backend: Keep Sys.opaque_identity in Cmm and Mach (port upstream PR 9412) (ocaml#238) faaf149 flambda-backend: Rename Un_cps -> To_cmm (ocaml#261) ecb0201 flambda-backend: Add "-dcfg" flag to ocamlopt (ocaml#254) 32ec58a flambda-backend: Bypass Simplify (ocaml#162) bd4ce4a flambda-backend: Revert "Semaphore without probes: dummy notes (ocaml#142)" (ocaml#242) c98530f flambda-backend: Semaphore without probes: dummy notes (ocaml#142) c9b6a04 flambda-backend: Remove hack for .depend from runtime/dune (ocaml#170) 6e5d4cf flambda-backend: Build and install Semaphore (ocaml#183) 924eb60 flambda-backend: Special constructor for %sys_argv primitive (ocaml#166) 2ac6334 flambda-backend: Build ocamldoc (ocaml#157) c6f7267 flambda-backend: Add -mbranches-within-32B to major_gc.c compilation (where supported) a99fdee flambda-backend: Merge pull request ocaml#10195 from stedolan/mark-prefetching bd72dcb flambda-backend: Prefetching optimisations for sweeping (ocaml#9934) 27fed7e flambda-backend: Add missing index param for Obj.field (ocaml#145) cd48b2f flambda-backend: Fix camlinternalOO at -O3 with Flambda 2 (ocaml#132) 9d85430 flambda-backend: Fix testsuite execution (ocaml#125) ac964ca flambda-backend: Comment out `[@inlined]` annotation. (ocaml#136) ad4afce flambda-backend: Fix magic numbers (test suite) (ocaml#135) 9b033c7 flambda-backend: Disable the comparison of bytecode programs (`ocamltest`) (ocaml#128) e650abd flambda-backend: Import flambda2 changes (`Asmpackager`) (ocaml#127) 14dcc38 flambda-backend: Fix error with Record_unboxed (bug in block kind patch) (ocaml#119) 2d35761 flambda-backend: Resurrect [@inline never] annotations in camlinternalMod (ocaml#121) f5985ad flambda-backend: Magic numbers for cmx and cmxa files (ocaml#118) 0e8b9f0 flambda-backend: Extend conditions to include flambda2 (ocaml#115) 99870c8 flambda-backend: Fix Translobj assertions for Flambda 2 (ocaml#112) 5106317 flambda-backend: Minor fix for "lazy" compilation in Matching with Flambda 2 (ocaml#110) dba922b flambda-backend: Oclassic/O2/O3 etc (ocaml#104) f88af3e flambda-backend: Wire in the remaining Flambda 2 flags (ocaml#103) 678d647 flambda-backend: Wire in the Flambda 2 inlining flags (ocaml#100) 1a8febb flambda-backend: Formatting of help text for some Flambda 2 options (ocaml#101) 9ae1c7a flambda-backend: First set of command-line flags for Flambda 2 (ocaml#98) bc0bc5e flambda-backend: Add config variables flambda_backend, flambda2 and probes (ocaml#99) efb8304 flambda-backend: Build our own ocamlobjinfo from tools/objinfo/ at the root (ocaml#95) d2cfaca flambda-backend: Add mutability annotations to Pfield etc. (ocaml#88) 5532555 flambda-backend: Lambda block kinds (ocaml#86) 0c597ba flambda-backend: Revert VERSION, etc. back to 4.12.0 (mostly reverts 822d0a0 from upstream 4.12) (ocaml#93) 037c3d0 flambda-backend: Float blocks 7a9d190 flambda-backend: Allow --enable-middle-end=flambda2 etc (ocaml#89) 9057474 flambda-backend: Root scanning fixes for Flambda 2 (ocaml#87) 08e02a3 flambda-backend: Ensure that Lifthenelse has a boolean-valued condition (ocaml#63) 77214b7 flambda-backend: Obj changes for Flambda 2 (ocaml#71) ecfdd72 flambda-backend: Cherry-pick 9432cfdadb043a191b414a2caece3e4f9bbc68b7 (ocaml#84) d1a4396 flambda-backend: Add a `returns` field to `Cmm.Cextcall` (ocaml#74) 575dff5 flambda-backend: CMM traps (ocaml#72) 8a87272 flambda-backend: Remove Obj.set_tag and Obj.truncate (ocaml#73) d9017ae flambda-backend: Merge pull request ocaml#80 from mshinwell/fb-backport-pr10205 3a4824e flambda-backend: Backport PR#10205 from upstream: Avoid overwriting closures while initialising recursive modules f31890e flambda-backend: Install missing headers of ocaml/runtime/caml (ocaml#77) 83516f8 flambda-backend: Apply node created for probe should not be annotated as tailcall (ocaml#76) bc430cb flambda-backend: Add Clflags.is_flambda2 (ocaml#62) ed87247 flambda-backend: Preallocation of blocks in Translmod for value let rec w/ flambda2 (ocaml#59) a4b04d5 flambda-backend: inline never on Gc.create_alarm (ocaml#56) cef0bb6 flambda-backend: Config.flambda2 (ocaml#58) ff0e4f7 flambda-backend: Pun labelled arguments with type constraint in function applications (ocaml#53) d72c5fb flambda-backend: Remove Cmm.memory_chunk.Double_u (ocaml#42) 9d34d99 flambda-backend: Install missing artifacts 10146f2 flambda-backend: Add ocamlcfg (ocaml#34) 819d38a flambda-backend: Use OC_CFLAGS, OC_CPPFLAGS, and SHAREDLIB_CFLAGS for foreign libs (#30) f98b564 flambda-backend: Pass -function-sections iff supported. (#29) e0eef5e flambda-backend: Bootstrap (#11 part 2) 17374b4 flambda-backend: Add [@@Builtin] attribute to Primitives (#11 part 1) 85127ad flambda-backend: Add builtin, effects and coeffects fields to Cextcall (#12) b670bcf flambda-backend: Replace tuple with record in Cextcall (#10) db451b5 flambda-backend: Speedups in Asmlink (#8) 2fe489d flambda-backend: Cherry-pick upstream PR#10184 from upstream, dynlink invariant removal (rev 3dc3cd7 upstream) d364bfa flambda-backend: Local patch against upstream: enable function sections in the Dune build 886b800 flambda-backend: Local patch against upstream: remove Raw_spacetime_lib (does not build with -m32) 1a7db7c flambda-backend: Local patch against upstream: make dune ignore ocamldoc/ directory e411dd3 flambda-backend: Local patch against upstream: remove ocaml/testsuite/tests/tool-caml-tex/ 1016d03 flambda-backend: Local patch against upstream: remove ocaml/dune-project and ocaml/ocaml-variants.opam 93785e3 flambda-backend: To upstream: export-dynamic for otherlibs/dynlink/ via the natdynlinkops files (still needs .gitignore + way of generating these files) 63db8c1 flambda-backend: To upstream: stop using -O3 in otherlibs/Makefile.otherlibs.common eb2f1ed flambda-backend: To upstream: stop using -O3 for dynlink/ 6682f8d flambda-backend: To upstream: use flambda_o3 attribute instead of -O3 in the Makefile for systhreads/ de197df flambda-backend: To upstream: renamed ocamltest_unix.xxx files for dune bf3773d flambda-backend: To upstream: dune build fixes (depends on previous to-upstream patches) 6fbc80e flambda-backend: To upstream: refactor otherlibs/dynlink/, removing byte/ and native/ 71a03ef flambda-backend: To upstream: fix to Ocaml_modifiers in ocamltest 686d6e3 flambda-backend: To upstream: fix dependency problem with Instruct c311155 flambda-backend: To upstream: remove threadUnix 52e6e78 flambda-backend: To upstream: stabilise filenames used in backtraces: stdlib/, otherlibs/systhreads/, toplevel/toploop.ml 7d08e0e flambda-backend: To upstream: use flambda_o3 attribute in stdlib 403b82e flambda-backend: To upstream: flambda_o3 attribute support (includes bootstrap) 65032b1 flambda-backend: To upstream: use nolabels attribute instead of -nolabels for otherlibs/unix/ f533fad flambda-backend: To upstream: remove Compflags, add attributes, etc. 49fc1b5 flambda-backend: To upstream: Add attributes and bootstrap compiler a4b9e0d flambda-backend: Already upstreamed: stdlib capitalisation patch 4c1c259 flambda-backend: ocaml#9748 from xclerc/share-ev_defname (cherry-pick 3e937fc) 00027c4 flambda-backend: permanent/default-to-best-fit (cherry-pick 64240fd) 2561dd9 flambda-backend: permanent/reraise-by-default (cherry-pick 50e9490) c0aa4f4 flambda-backend: permanent/gc-tuning (cherry-pick e9d6d2f) git-subtree-dir: ocaml git-subtree-split: 23a7f73
Preamble
This pull request contains a submodule which at present refers to a commit which has not yet been merged into FlexDLL (see ocaml/flexdll#14). If you wish to experiment with the PR, you need to do one of two things:
git submodule update --init
, ignore the error message and then from theflexdll
directory rungit remote add -f dra27 https://github.com/dra27/flexdll.git
followed bygit checkout visual-studio
git submodule init
, edit.git/config
and change the URL for the flexdll repository to mine and then rungit submodule update
Obviously, notwithstanding other reasons, this PR should not be merged until ocaml/flexdll#14 is resolved.
Abstract (!)
This PR provides the option of bootstrapping FlexDLL from sources as part of the general build process for native Windows OCaml. At present, this support is not offered for Cygwin (see below).
The philosophical arguments for doing this have already been aired on at least one occasion, but there are two more concrete reasons for the desirability of being able to do this:
Cygwin
Cygwin OCaml uses FlexDLL to enable shared libraries - some of the changes I have made are in fact porting existing functionality in the Unix build system to allow Cygwin flexlink to be bootstrapped, because there is/was no binary distribution. I have not extended this full bootstrapping support to Cygwin yet for two reasons:
configure
rather thanMakefile
-editing route.At some point in the coming months, if nobody else gets there first, I shall contribute a unified build system for the native Windows ports via
configure
and I would like to propose that full bootstrapping for Cygwin of FlexDLL be left until that work is done (by whoever).Commentary
Overall, it is pleasing (to me, at least) how few changes are required to facilitate this - which I hope may aid in its acceptance.
The eight commits include an earlier partial effort (09feadf and 29f7d7d). This simpler approach allowed the user to say
which allowed a byte-code flexlink to be compiled and pre-installed. However, it still assumes that the user has set their
PATH
appropriately so that that flexlink can then be used in the main build process for OCaml.In that earlier work, the flexdll sources have to downloaded (either by source tarball or by git clone) and placed into the OCaml source tree prior to issuing
make -f Makefile.nt flexdll
. However, this is far from the best way for Git - both the fact that OCaml and FlexDLL are truly separate projects and the fact that it remains possible to compile OCaml without also compiling FlexDLL makes the repository a perfect candidate for a Git submodule which is what is then added in d8a35be. Error messages inMakefile.nt
are added which alert the user to the absence of flexlink and advise how to fix the problem (this is part of the reason for not incorporating this to Cygwin at the moment, as such testing really should be inconfigure
).In order to allow the OCaml build procedure to use a locally bootstrapped copy of FlexDLL, it is necessary to add one feature to the compilers and to
ocamlmklib
- there needs to be a way to override theflexlink
command (or, more specifically,Config.mkexe
,Config.mkdll
andConfig.mkmaindll
). The only alternative I could see would be to build two copies of the compiler andocamlmklib
which I don't think is an acceptable burden to add to the build process - both in terms of time and complexity of understanding. My solution instead in f4fd8b1 is to split theFLEXLINK
Makefile variable into known constituent parts and then to allow it to be overridden at runtime by anOCAML_FLEXLINK
environment variable (i.e. the way in whichmkexe
,mkdll
andmkmaindll
is less general than it was before).Finally, 9b62404 utilises this new feature to allow a bootstrapped FlexDLL to be used if
flexlink
is not found inPATH
. It also amends theopt.opt
target to build a native code version offlexlink.exe
only if FlexDLL was bootstrapped in the first place and theinstallbyt
target also installsflexlink.exe
and the required FlexDLL object files and manifests again, only if FlexDLL was bootstrapped in the first place. The change to the build process principally involves communicating either a revisedOCAMLOPT
variable or aOCAML_FLEXLINK=...
line to be used, similar to the overriding already in various places to specify the boot compiler. At present, the expressions used are more verbose than would be ideal (especially the need to translate forward slashes to backslashes), however this would be vastly simplified by the use of aconfigure
script for the native Windows ports, so I would crave your patience with the very long$(if $(shell ... ))
lines!This has been tested with all four native Windows ports (running
make -f Makefile.nt [flexdll] world bootstrap opt opt.opt install
) both with and without the submodule initialised and in all eight builds the test-suite (utilising various patches pending in #366 and #384) runs with 100% pass rate.I haven't updated
README.win32.adoc
with instructions because I can't be bothered to update a guide which I'm already planning on completely rewriting once I've finished the Visual Studio testing which has been yielding all these patches...Thank you for getting to the bottom 😄