Stop before emit

[gretay-js]

This PR extends the existing command-line option "-stop-after " to accept the argument "scheduling" in addition to the existing "parsing" and "typing" which were added in #1945.

With "-stop-after scheduling", the native compiler will stop compilation after scheduling pass. It will produce .cmi and .cmx files as usual, and perform the optimizations in the backend, but won't emit
assembly or object files, and won't perform linking, packing, etc. The bytecode compiler will not accept "-stop-after scheduling".

It may be more accurate to call the option "-stop-before emit" rather than "-stop-after scheduling", but it doesn't fit with the existing option's naming convention.

Clflags.Compiler_pass.pass_names is extended to take a filter function as an argument, in preparation for another command-line option I'd like to propose shortly, in a separate PR.

[gretay-js]

ping.

[Octachron]

Looking at the test, I am not sure if it make sense to test the -S -stop-after scheduling case. The two options are contradictory, and it might be better to not rely on the behavior of ocamlopt in this situation.

Also, I am not sure what is the opinion on @shindere on the new asmext ocamltest's variable.

[gretay-js]

Looking at the test, I am not sure if it make sense to test the -S -stop-after scheduling case. The two options are contradictory, and it might be better to not rely on the behavior of ocamlopt in this situation.

This test was to check that with -stop-after scheduling, cmx is generated but assembly and object files aren't, that's why I added "-S" option.

Also, I am not sure what is the opinion on @shindere on the new asmext ocamltest's variable.

It already has objext.

[shindere]

Greta Yorsh (2019/10/07 03:29 -0700):

> Looking at the test, I am not sure if it make sense to test the `-S -stop-after scheduling` case. The two options are contradictory, and it might be better to not rely on the behavior of `ocamlopt` in this situation. This test was to check that with -stop-after scheduling, cmx is generated but assembly and object files aren't, that's why I added "-S" option. > Also, I am not sure what is the opinion on @shindere on the new asmext ocamltest's variable. It already has objext.

I am supposed not to work until next monday so you if you're not in a hurry you may wait till then if you want me to have a look but if you wish this to be merged promptly I'm fine.

[gretay-js]

@shindere can you please have a look at the new tests and the changes in ocamltest?

[Octachron]

The implementation in Asmgen seems ok.
I am not so fond of the Clflags interface: I would rather complexify the API in the PR that makes use of this complexity.

[shindere]

Greta Yorsh (2019/10/07 03:09 -0700):

thank you! Fixed. I found another one in an unrelated tests/unwind/check-linker-version.sh and fixed it too while at it. Is it ok to include this small unrelated change?

As far as I am concerned yes, definitely. You could put this fix in a distinct commit but I'd say even that is not mandatory.

[shindere]

Greta Yorsh (2019/10/23 03:34 -0700):

@shindere can you please have a look at the new tests and the changes in ocamltest?

I'm very okay with everything, thanks! One thing you may want to do in the test scripts is to provide a reason for test failure. You can do that like this: ```bash echo "afl-showmap not available" > ${ocamltest_response} ```

[shindere]

Florian Angeletti (2019/10/25 05:06 -0700):

Sorry if I wasn't clear; but you should remove the currently meaningless filter argument from the implementation. The three line changes should go to the PR that need them, to avoid coupling the PR reviews.

Yes, I fully agree with this. If for some reason your upcoming PRs don't get merge, then this change should not be merged either. Also, as much as possible, the PRs should be independent of each other so that one can be reverted if necessary.

[gretay-js]

I removed the filter function from this PR. Thank you for clarifying this point!

[gretay-js]

One thing you may want to do in the test scripts is to provide a reason for test failure. You can do that like this: bash echo "afl-showmap not available" > ${ocamltest_response}

cool, I didn't know we could do that, thanks! I updated the test.

[shindere]

Greta Yorsh (2019/10/28 04:00 -0700):

> One thing you may want to do in the test scripts is to provide a reason for test failure. You can do that like this: ```bash echo "afl-showmap not available" > ${ocamltest_response} ``` cool, I didn't know we could do that, thanks! I updated the test.

Thanks! Now perhaps you can cleanup the history? :)

[gretay-js]

Rebased, cleaned history, and add Changes entry.

[shindere]

Thanks! I'll leave the merge to somebody else because I just revieweed the ocamltest-related part.

[Octachron]

One last nitpick, it is better to keep an exhaustive list of Clflags.Compiler_flags to remind the reader of all possible interactions.

[gretay-js]

Thank you! This check was also missing from optmain.
I added an assertion because "stop_after p" where p.is_compilation_pass=true implies compile_only=true, but if we ever have p such that p.is_compilation_pass=false, then the length calculation above would need to be updated. Please let me know if it's okay, and I'll need to clean the history again.

[Octachron]

(The new test is failing on Windows/Appveyor)

[gretay-js]

sorry I haven't noticed earlier. Checking.

[gretay-js]

The test fails on windows, because the path to the build artifacts is incorrectly formed with a mix of '/' and '\' instead of '\' only.
... testing 'stop_after_scheduling.ml' with 1.1.1.1.1 (script) => failed (not found expected C:\projects\🐫реализация-mingw32\testsuite\_ocamltest\tests/tool-ocamlopt-stop-after\stop_after_scheduling\ocamlopt.byte/stop_after_scheduling.cmx)

Wrong separators come not only from my script - some of them are generated by ocamltest internally for the variable test_build_directory. I can't figure out how to fix it. Also, strangely, there are other tests that use a similar pattern to form filenames and they pass on windows. By the way, it looks like test_source_directory is formed correctly, but not test_build_directory.

Is okay to just disable this test windows (unless @dra27 @shindere have a fix)?